From 5d5b7e14d41e00671fc938ad0db7714d56bd455e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 27 Apr 2020 10:41:49 +0100 Subject: [PATCH] LUCENE-9314: Use SingletonDocumentBatch in monitor when we only have a single document --- lucene/CHANGES.txt | 3 + .../apache/lucene/monitor/DocumentBatch.java | 12 +++- .../lucene/monitor/TestDocumentBatch.java | 58 +++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 lucene/monitor/src/test/org/apache/lucene/monitor/TestDocumentBatch.java diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 9eb9ba54fec..9754bbeee45 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -210,6 +210,9 @@ Bug Fixes finishing threads fail to pick up pending merges causing potential thread starvation on forceMerge calls. (Simon Willnauer) +* LUCENE-9314: Single-document monitor runs were using the less efficient MultiDocumentBatch + implementation. (Pierre-Luc Perron, Alan Woodward) + Other --------------------- diff --git a/lucene/monitor/src/java/org/apache/lucene/monitor/DocumentBatch.java b/lucene/monitor/src/java/org/apache/lucene/monitor/DocumentBatch.java index 69119ef66c5..0ca58c9e9dc 100644 --- a/lucene/monitor/src/java/org/apache/lucene/monitor/DocumentBatch.java +++ b/lucene/monitor/src/java/org/apache/lucene/monitor/DocumentBatch.java @@ -49,11 +49,18 @@ abstract class DocumentBatch implements Closeable, Supplier { /** * Create a DocumentBatch containing a set of InputDocuments * - * @param docs Collection of documents to add + * @param docs Collection of documents to add. There must be at least one + * document in the collection. * @return the batch containing the input documents */ public static DocumentBatch of(Analyzer analyzer, Document... docs) { - return new MultiDocumentBatch(analyzer, docs); + if (docs.length == 0) { + throw new IllegalArgumentException("A DocumentBatch must contain at least one document"); + } else if (docs.length == 1) { + return new SingletonDocumentBatch(analyzer, docs[0]); + } else { + return new MultiDocumentBatch(analyzer, docs); + } } // Implementation of DocumentBatch for collections of documents @@ -63,6 +70,7 @@ abstract class DocumentBatch implements Closeable, Supplier { private final LeafReader reader; MultiDocumentBatch(Analyzer analyzer, Document... docs) { + assert(docs.length > 0); IndexWriterConfig iwc = new IndexWriterConfig(analyzer); try (IndexWriter writer = new IndexWriter(directory, iwc)) { this.reader = build(writer, docs); diff --git a/lucene/monitor/src/test/org/apache/lucene/monitor/TestDocumentBatch.java b/lucene/monitor/src/test/org/apache/lucene/monitor/TestDocumentBatch.java new file mode 100644 index 00000000000..b82a0ede430 --- /dev/null +++ b/lucene/monitor/src/test/org/apache/lucene/monitor/TestDocumentBatch.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.monitor; + +import java.io.IOException; + +import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.document.Document; +import org.apache.lucene.util.LuceneTestCase; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; + +public class TestDocumentBatch extends LuceneTestCase { + + public static final Analyzer ANALYZER = new StandardAnalyzer(); + + @Test(expected = IllegalArgumentException.class) + public void testDocumentBatchThrowsIllegalArgumentExceptionUponZeroDocument() { + DocumentBatch.of(ANALYZER); + } + + public void testSingleDocumentAndArrayOfOneDocumentResultInSameDocumentBatch() throws IOException { + Document doc = new Document(); + try (DocumentBatch batchDoc = DocumentBatch.of(ANALYZER, doc); + DocumentBatch batchArr = DocumentBatch.of(ANALYZER, new Document[] {doc})) { + assertThat(batchDoc.getClass().getName(), containsString("SingletonDocumentBatch")); + assertEquals(batchDoc.getClass(), batchArr.getClass()); + } + } + + public void testDocumentBatchClassDiffersWhetherItContainsOneOrMoreDocuments() throws IOException { + Document doc = new Document(); + try (DocumentBatch batch1 = DocumentBatch.of(ANALYZER, new Document[] {doc}); + DocumentBatch batch2 = DocumentBatch.of(ANALYZER, doc, doc); + DocumentBatch batch3 = DocumentBatch.of(ANALYZER, doc, doc, doc)) { + assertNotEquals(batch1.getClass(), batch2.getClass()); + assertEquals(batch2.getClass(), batch3.getClass()); + assertThat(batch3.getClass().getName(), containsString("MultiDocumentBatch")); + } + } +}