You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/06/27 22:11:37 UTC
[GitHub] [lucene-solr] s1monw opened a new pull request #1623: LUCENE-8962: Merge segments on getReader
s1monw opened a new pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623
Add IndexWriter merge-on-refresh feature to selectively merge small segments on getReader, subject to a configurable timeout, to improve search performance by reducing the number of small segments for searching.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r470869880
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
Review comment:
Thank you for good `success` variable naming instead of the usual `success1` and `success2` etc.!
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3335,49 +3416,60 @@ public void mergeFinished(boolean committed, boolean segmentDropped) throws IOEx
// includedInCommit will be set (above, by our caller) to false if the allowed max wall clock
// time (IWC.getMaxCommitMergeWaitMillis()) has elapsed, which means we did not make the timeout
// and will not commit our merge to the to-be-commited SegmentInfos
-
if (segmentDropped == false
&& committed
- && includeInCommit.get()) {
+ && includeMergeResult.get()) {
+
+ // make sure onMergeComplete really was called:
+ assert origInfo != null;
if (infoStream.isEnabled("IW")) {
infoStream.message("IW", "now apply merge during commit: " + toWrap.segString());
}
- // make sure onMergeComplete really was called:
- assert origInfo != null;
-
- deleter.incRef(origInfo.files());
+ if (trigger == MergeTrigger.COMMIT) { // if we do this in a getReader call here this is obsolete
+ deleter.incRef(origInfo.files());
Review comment:
Hmm why don't we need to `deleter.incRef` for NRT reader case? I guess the NRT reader we opened holds a reference already?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -630,6 +694,64 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
return r;
}
+ private StandardDirectoryReader finishGetReaderMerge(AtomicBoolean hasTimedOut, Map<String, SegmentReader> mergedReaders,
+ Map<String, SegmentReader> openedReadOnlyClones, SegmentInfos openingSegmentInfos,
+ boolean applyAllDeletes, boolean writeAllDeletes,
+ MergePolicy.MergeSpecification onCommitMerges, long maxCommitMergeWaitMillis) throws IOException {
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
+ hasTimedOut.set(true);
Review comment:
Hmm, what if the merges finished before the timeout? The `await` would return early, and return `true` if it did not timeout (I think?). Maybe we do not need/care to distinguish that? In which case maybe renamed `hasTimedOut` to something else (`mergesFinished`?).
##########
File path: lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java
##########
@@ -82,7 +82,8 @@ protected DirectoryReader doBody(String segmentFileName) throws IOException {
}
/** Used by near real-time search */
- static DirectoryReader open(IndexWriter writer, SegmentInfos infos, boolean applyAllDeletes, boolean writeAllDeletes) throws IOException {
+ static StandardDirectoryReader open(IndexWriter writer, IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFunction,
+ SegmentInfos infos, boolean applyAllDeletes, boolean writeAllDeletes) throws IOException {
Review comment:
Re-indent?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
+ includeMergeReader.set(false);
+ boolean openNewReader = mergedReaders.isEmpty() == false;
+ if (openNewReader) {
+ StandardDirectoryReader mergedReader = StandardDirectoryReader.open(this,
+ sci -> {
+ // as soon as we remove the reader and return it the StandardDirectoryReader#open
+ // will take care of closing it. We only need to handle the readers that remain in the
+ // mergedReaders map and close them.
+ SegmentReader remove = mergedReaders.remove(sci.info.name);
+ if (remove == null) {
+ remove = openedReadOnlyClones.remove(sci.info.name);
+ assert remove != null;
+ // each of the readers we reuse from the previous reader needs to be refInced
+ // since we reuse them but don't have an implicit refInc in the SDR:open call
Review comment:
s/`refInc`/`incRef`?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
+ includeMergeReader.set(false);
+ boolean openNewReader = mergedReaders.isEmpty() == false;
+ if (openNewReader) {
+ StandardDirectoryReader mergedReader = StandardDirectoryReader.open(this,
+ sci -> {
+ // as soon as we remove the reader and return it the StandardDirectoryReader#open
+ // will take care of closing it. We only need to handle the readers that remain in the
+ // mergedReaders map and close them.
+ SegmentReader remove = mergedReaders.remove(sci.info.name);
+ if (remove == null) {
+ remove = openedReadOnlyClones.remove(sci.info.name);
+ assert remove != null;
+ // each of the readers we reuse from the previous reader needs to be refInced
Review comment:
s/`refInced`/`incRef'd`?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3335,49 +3416,60 @@ public void mergeFinished(boolean committed, boolean segmentDropped) throws IOEx
// includedInCommit will be set (above, by our caller) to false if the allowed max wall clock
// time (IWC.getMaxCommitMergeWaitMillis()) has elapsed, which means we did not make the timeout
// and will not commit our merge to the to-be-commited SegmentInfos
-
if (segmentDropped == false
&& committed
- && includeInCommit.get()) {
+ && includeMergeResult.get()) {
+
+ // make sure onMergeComplete really was called:
+ assert origInfo != null;
if (infoStream.isEnabled("IW")) {
infoStream.message("IW", "now apply merge during commit: " + toWrap.segString());
}
- // make sure onMergeComplete really was called:
- assert origInfo != null;
-
- deleter.incRef(origInfo.files());
+ if (trigger == MergeTrigger.COMMIT) { // if we do this in a getReader call here this is obsolete
+ deleter.incRef(origInfo.files());
+ }
Set<String> mergedSegmentNames = new HashSet<>();
for (SegmentCommitInfo sci : segments) {
mergedSegmentNames.add(sci.info.name);
}
List<SegmentCommitInfo> toCommitMergedAwaySegments = new ArrayList<>();
- for (SegmentCommitInfo sci : committingSegmentInfos) {
+ for (SegmentCommitInfo sci : mergingSegmentInfos) {
if (mergedSegmentNames.contains(sci.info.name)) {
toCommitMergedAwaySegments.add(sci);
- deleter.decRef(sci.files());
+ if (trigger == MergeTrigger.COMMIT) { // if we do this in a getReader call here this is obsolete
+ deleter.decRef(sci.files());
+ }
}
}
// Construct a OneMerge that applies to toCommit
MergePolicy.OneMerge applicableMerge = new MergePolicy.OneMerge(toCommitMergedAwaySegments);
applicableMerge.info = origInfo;
long segmentCounter = Long.parseLong(origInfo.info.name.substring(1), Character.MAX_RADIX);
- committingSegmentInfos.counter = Math.max(committingSegmentInfos.counter, segmentCounter + 1);
- committingSegmentInfos.applyMergeChanges(applicableMerge, false);
+ mergingSegmentInfos.counter = Math.max(mergingSegmentInfos.counter, segmentCounter + 1);
+ mergingSegmentInfos.applyMergeChanges(applicableMerge, false);
} else {
if (infoStream.isEnabled("IW")) {
infoStream.message("IW", "skip apply merge during commit: " + toWrap.segString());
}
}
- toWrap.mergeFinished(committed, false);
+ toWrap.mergeFinished(committed, segmentDropped);
super.mergeFinished(committed, segmentDropped);
}
@Override
- void onMergeComplete() {
- // clone the target info to make sure we have the original info without the updated del and update gens
- origInfo = info.clone();
+ void onMergeComplete() throws IOException {
+ if (includeMergeResult.get()
+ && isAborted() == false
+ && info.info.maxDoc() > 0/* never do this if the segment if dropped / empty */) {
+ assert Thread.holdsLock(IndexWriter.this);
Review comment:
Maybe move the `assert` to top of the method? We should always hold IW's monitor lock on entry?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean hasTimedOut = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
+ return segmentReader;
+ } finally {
+ release(rld);
+ }
+ };
+ SegmentInfos openingSegmentInfos = null;
+ final long maxFullFlushMergeWaitMillis = config.getMaxFullFlushMergeWaitMillis();
boolean success2 = false;
try {
+ /* this is the essential part of the getReader method. We need to take care of the following things:
Review comment:
Thank you for these awesome details!!
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean hasTimedOut = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
+ return segmentReader;
+ } finally {
+ release(rld);
+ }
+ };
+ SegmentInfos openingSegmentInfos = null;
+ final long maxFullFlushMergeWaitMillis = config.getMaxFullFlushMergeWaitMillis();
boolean success2 = false;
try {
+ /* this is the essential part of the getReader method. We need to take care of the following things:
+ * - flush all currently in-memory DWPTs to disk
+ * - apply all deletes & updates to new and to the existing DWPTs
+ * - prevent flushes and applying deletes of concurrently indexing DWPTs to be applied
+ * - open a SDR on the updated SIS
+ *
+ * in order do prevent concurrent flushes we call DocumentsWriter#flushAllThreads that swaps out the deleteQueue
Review comment:
s/`do`/`to`?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3244,7 +3322,7 @@ private long prepareCommitInternal() throws IOException {
if (anyChanges && maxCommitMergeWaitMillis > 0) {
// we can safely call prepareOnCommitMerge since writeReaderPool(true) above wrote all
// necessary files to disk and checkpointed them.
- onCommitMerges = prepareOnCommitMerge(toCommit, includeInCommit);
+ onCommitMerges = preparePointInTimeMerge(toCommit, includeInCommit, MergeTrigger.COMMIT, sci->{});
Review comment:
Good name! Maybe rename `onCommitMerges` to `pointInTimeMerges`?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -630,6 +694,64 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
return r;
}
+ private StandardDirectoryReader finishGetReaderMerge(AtomicBoolean hasTimedOut, Map<String, SegmentReader> mergedReaders,
+ Map<String, SegmentReader> openedReadOnlyClones, SegmentInfos openingSegmentInfos,
+ boolean applyAllDeletes, boolean writeAllDeletes,
+ MergePolicy.MergeSpecification onCommitMerges, long maxCommitMergeWaitMillis) throws IOException {
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
Review comment:
Maybe move this `assert` to top of method?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
Maybe we need to rename this `IWC` option and variables? Maybe we need two timeouts, one for `commit`, one for `getReader`? Or, maybe we somehow make this a property of the `MergeSpecification` so `MergePolicy` can decide case by case what the timeout should be?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw merged pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw merged pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-675459732
Another failure that reproduces only on the PR:
```
[junit4:pickseed] Seed property 'tests.seed' already defined: CADC6D7945159855
[junit4] <JUnit4> says jolly good day! Master seed: CADC6D7945159855
[junit4] Executing 1 suite with 1 JVM.
[junit4]
[junit4] Started J0 PID(1047056@localhost).
[junit4] Suite: org.apache.lucene.spatial.prefix.NumberRangeFacetsTest
[junit4] OK 0.37s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:428852A3EBE9F5AD]}
[junit4] OK 0.07s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:F6DEEE5FDF2B3E81]}
[junit4] OK 0.02s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:783778838EEF764A]}
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=NumberRangeFacetsTest -Dtests.method=test -Dtests.seed=CADC6D7945159855 -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=lu-CD -Dtests.\
timezone=America/Iqaluit -Dtests.asserts=true -Dtests.file.encoding=UTF-8
[junit4] ERROR 0.10s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:49D9D366E2112D63]} <<<
[junit4] > Throwable #1: java.nio.file.NoSuchFileException: _3.fdx
[junit4] > at org.apache.lucene.store.ByteBuffersDirectory.deleteFile(ByteBuffersDirectory.java:148)
[junit4] > at org.apache.lucene.store.MockDirectoryWrapper.deleteFile(MockDirectoryWrapper.java:607)
[junit4] > at org.apache.lucene.store.LockValidatingDirectoryWrapper.deleteFile(LockValidatingDirectoryWrapper.java:38)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.deleteFile(IndexFileDeleter.java:696)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.deleteFiles(IndexFileDeleter.java:690)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:589)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:620)
[junit4] > at org.apache.lucene.index.IndexWriter.decRefDeleter(IndexWriter.java:5354)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.lambda$doClose$1(StandardDirectoryReader.java:370)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.doClose(StandardDirectoryReader.java:384)
[junit4] > at org.apache.lucene.index.IndexReader.decRef(IndexReader.java:244)
[junit4] > at org.apache.lucene.index.IndexReader.close(IndexReader.java:385)
[junit4] > at org.apache.lucene.util.IOUtils.close(IOUtils.java:89)
[junit4] > at org.apache.lucene.util.IOUtils.close(IOUtils.java:77)
[junit4] > at org.apache.lucene.spatial.SpatialTestCase.commit(SpatialTestCase.java:97)
[junit4] > at org.apache.lucene.spatial.prefix.NumberRangeFacetsTest.test(NumberRangeFacetsTest.java:92)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)Throwable #2: java.lang.RuntimeException: MockDirectoryWrapper: cannot close: there are still 2 open files: {_5.cfs=1, _4.cfs=1}
[junit4] > at org.apache.lucene.store.MockDirectoryWrapper.close(MockDirectoryWrapper.java:812)
[junit4] > at org.apache.lucene.util.IOUtils.close(IOUtils.java:89)
[junit4] > at org.apache.lucene.util.IOUtils.close(IOUtils.java:77)
[junit4] > at org.apache.lucene.spatial.SpatialTestCase.tearDown(SpatialTestCase.java:72)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] > Caused by: java.lang.RuntimeException: unclosed IndexInput: _4.cfs
[junit4] > at org.apache.lucene.store.MockDirectoryWrapper.addFileHandle(MockDirectoryWrapper.java:730)
[junit4] > at org.apache.lucene.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:773)
[junit4] > at org.apache.lucene.codecs.lucene50.Lucene50CompoundReader.<init>(Lucene50CompoundReader.java:77)
[junit4] > at org.apache.lucene.codecs.lucene50.Lucene50CompoundFormat.getCompoundReader(Lucene50CompoundFormat.java:71)
[junit4] > at org.apache.lucene.index.SegmentCoreReaders.<init>(SegmentCoreReaders.java:101)
[junit4] > at org.apache.lucene.index.SegmentReader.<init>(SegmentReader.java:83)
[junit4] > at org.apache.lucene.index.ReadersAndUpdates.getReader(ReadersAndUpdates.java:171)
[junit4] > at org.apache.lucene.index.ReadersAndUpdates.getReadOnlyClone(ReadersAndUpdates.java:213)
[junit4] > at org.apache.lucene.index.IndexWriter.lambda$getReader$0(IndexWriter.java:569)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.open(StandardDirectoryReader.java:105)
[junit4] > at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:625)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.doOpenFromWriter(StandardDirectoryReader.java:290)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:265)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.doOpenIfChanged(StandardDirectoryReader.java:255)
[junit4] > at org.apache.lucene.index.DirectoryReader.openIfChanged(DirectoryReader.java:140)
[junit4] > at org.apache.lucene.spatial.SpatialTestCase.commit(SpatialTestCase.java:95)
[junit4] > at org.apache.lucene.spatial.prefix.NumberRangeFacetsTest.test(NumberRangeFacetsTest.java:92)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > ... 28 more
[junit4] OK 0.05s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:51856CB4319EDD8]}
[junit4] OK 0.04s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:94E285943EA5A0D8]}
[junit4] OK 0.04s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:AA3CE1122C95B0DE]}
[junit4] OK 0.06s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:368F7B680F812470]}
[junit4] OK 0.07s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:4239E06784A332A]}
[junit4] OK 0.03s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:D3A8C8BC1C1D0478]}
[junit4] OK 0.06s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:26E920E7CEA126A0]}
[junit4] OK 0.07s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:AD4EEC224A3C8769]}
[junit4] OK 0.04s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:CA7D799BAF414598]}
[junit4] OK 0.05s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:A5365E8433D4C4E8]}
[junit4] OK 0.05s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:F8A8511CE1A582B1]}
[junit4] OK 0.01s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:9B1ABC18F3271780]}
[junit4] OK 0.03s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:1DE11BD1382F7CE9]}
[junit4] OK 0.01s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:A9AEC9CAF41A0E9B]}
[junit4] OK 0.07s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:B6DAB6C4888FE199]}
[junit4] OK 0.05s | NumberRangeFacetsTest.test {seed=[CADC6D7945159855:BC4AC5282267A734]}
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene86): {dateRange=PostingsFormat(name=MockRandom), id=Lucene84}, docValues:{}, maxPointsInLeafNode=1082, maxMBSortInHeap=5.908257958947349, si\
m=Asserting(RandomSimilarity(queryNorm=true): {id=ClassicSimilarity}), locale=lu-CD, timezone=America/Iqaluit
[junit4] 2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=464866264,total=536870912
[junit4] 2> NOTE: All tests run in this JVM: [NumberRangeFacetsTest]
[junit4] Completed [1/1 (1!)] in 1.48s, 20 tests, 1 error <<< FAILURES!
[junit4]
[junit4]
[junit4] Tests with failures [seed: CADC6D7945159855]:
[junit4] - org.apache.lucene.spatial.prefix.NumberRangeFacetsTest.test {seed=[CADC6D7945159855:49D9D366E2112D63]}
[junit4]
[junit4]
[junit4] JVM J0: 0.34 .. 2.23 = 1.89s
[junit4] Execution time total: 2.23 sec.
[junit4] Tests summary: 1 suite, 20 tests, 1 error
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msfroh commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
msfroh commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-673730625
I went through the code yesterday to try to understand what's going on.
I'm not familiar with the pooled reader stuff, but it seems reasonable to me. I trust the tests (especially w/ added test coverage) far more than I trust my knowledge of that part of the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-651167468
A different, reproducing, test failure, likely from the same cause:
```
> java.lang.AssertionError
> at __randomizedtesting.SeedInfo.seed([C64EB0BA0CE0061F:FB961E96340E586F]:0)
> at org.apache.lucene.index.IndexWriter.maybeCloseOnTragicEvent(IndexWriter.java:5026)
> at org.apache.lucene.index.IndexWriter.tragicEvent(IndexWriter.java:5019)
> at org.apache.lucene.index.IndexWriter.merge(IndexWriter.java:4214)
> at org.apache.lucene.index.IndexWriter$IndexWriterMergeSource.merge(IndexWriter.java:5735)
> at org.apache.lucene.index.SerialMergeScheduler.merge(SerialMergeScheduler.java:40)
> at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:581)
> at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:103)
> at org.apache.lucene.index.TestIndexWriterExceptions2.testBasics(TestIndexWriterExceptions2.java:205)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1754)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:942)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:978)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:992)
> at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:49)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
> at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:48)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:819)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:470)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:951)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:836)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:887)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:898)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:41)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
> at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:54)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:826)
> at java.base/java.lang.Thread.run(Thread.java:834)
2> NOTE: reproduce with: ant test -Dtestcase=TestIndexWriterExceptions2 -Dtests.method=testBasics -Dtests.seed=C64EB0BA0CE0061F -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=d\
e-BE -Dtests.timezone=Atlantic/Azores -Dtests.asserts=true -Dtests.file.encoding=UTF-8
2> NOTE: test params are: codec=Asserting(Lucene86): {}, docValues:{}, maxPointsInLeafNode=1532, maxMBSortInHeap=7.7106929429614635, sim=Asserting(RandomSimilarity(queryNorm=false): {tex\
t_payloads=LM Dirichlet(2000.000000), text_vectors=IB SPL-D3(800.0), text1=IB SPL-DZ(0.3)}), locale=de-BE, timezone=Atlantic/Azores
2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=245301416,total=268435456
2> NOTE: All tests run in this JVM: [TestRollingUpdates, Test2BPostingsBytes, TestIndexWriterExceptions2]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471958847
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean hasTimedOut = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
+ return segmentReader;
+ } finally {
+ release(rld);
+ }
+ };
+ SegmentInfos openingSegmentInfos = null;
+ final long maxFullFlushMergeWaitMillis = config.getMaxFullFlushMergeWaitMillis();
boolean success2 = false;
try {
+ /* this is the essential part of the getReader method. We need to take care of the following things:
Review comment:
yeah I think it makes sense to have these details in these complex methods
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674108890
> I'm not familiar with the pooled reader stuff, but it seems reasonable to me. I trust the tests (especially w/ added test coverage) far more than I trust my knowledge of that part of the code.
@msfroh we maintain a pool of readers and their corresponding updates in the IndexWriter in order to reuse them in NRT readers, for applying deletes and DV updates or as merge sources. The key here is that the core reader is immutable and we maintain a reference to it even if DV or delete generations change. For this change I borrow the reader from the pool the the time being and once the merge is done I pull a reference to the freshly created segment ie. it's core reader with DV and delete generations being `0` That reader is then used to open the directory reader on top of the merged and remaining segments
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471174298
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
OK, I misunderstood the usage of `includeMergeReader`. I guess the idea is that we let them continue, but keep the original (merging) segments in the reader we're opening, so we do need a separate condition for that
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dnhatn commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
dnhatn commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674635878
@s1monw I like your first commit :). I wonder if we can continue that approach by moving the merge logic outside `fullFlushLock` and reopen a new reader if the merge completes within the timeout? But, I might be missing something obvious here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674718362
> @s1monw I like your first commit :). I wonder if we can continue that approach by moving the merge logic outside fullFlushLock and reopen a new reader if the merge completes within the timeout? But, I might be missing something obvious here.
hey @dnhatn I with we could! I first wrote a longish reply to your comment but then decided to push some documentation on how getReader works and why we can't do things outside of the lock, or at least not easily. Lemme know if it helps.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471955980
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
Review comment:
👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-678311047
@mikemccand I think we are ready here WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471017030
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
There's no need to check whether we timed out here, since we effectively abort all of the point-in-time merges we created by setting `includeMergeReader` to `false` below, right? I wonder if it would be cleaner to eliminate this AtomicBoolean that is shared with this and the merge threads, and instead using the existing merge abort technique that we have? OTOH IDK how that other mechanism works - is it aborting *all* outstanding merges?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
+ includeMergeReader.set(false);
+ boolean openNewReader = mergedReaders.isEmpty() == false;
+ if (openNewReader) {
+ StandardDirectoryReader mergedReader = StandardDirectoryReader.open(this,
+ sci -> {
+ // as soon as we remove the reader and return it the StandardDirectoryReader#open
+ // will take care of closing it. We only need to handle the readers that remain in the
+ // mergedReaders map and close them.
+ SegmentReader remove = mergedReaders.remove(sci.info.name);
+ if (remove == null) {
+ remove = openedReadOnlyClones.remove(sci.info.name);
+ assert remove != null;
+ // each of the readers we reuse from the previous reader needs to be refInced
+ // since we reuse them but don't have an implicit refInc in the SDR:open call
+ remove.incRef();
+ }
+ return remove;
+ }, openingSegmentInfos, applyAllDeletes, writeAllDeletes);
+ try {
+ r.close(); // close and swap in the new reader... close is cool here since we didn't leak this reader yet
+ } finally {
+ r = mergedReader;
+ }
+ }
+ }
+ replaceReaderSuccess = true;
+ } finally {
+ synchronized (this) {
Review comment:
and this one, closeMergedReaders?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
Review comment:
This method is getting pretty big, and it might help readability if we named these synchronized blocks as functions. This one could be replaceReader()? OTOH maybe it requires too many parameters - it's hard to tell in code review
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-673725282
> I kicked off beasting of all Lucene (core + modules) tests with this change ... no failures yet after 31 iterations.
OK, it has run 1061 iterations now of all Lucene (core + modules) tests, and some interesting (~15) failures:
```
ant test -Dtestcase=TestIndexWriter -Dtests.method=testRandomOperations -Dtests.seed=432EB011B0898067 -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=pt-ST -Dtests.timezone=US/Mountain -Dtests.asserts=true -Dtest\
s.file.encoding=UTF-8
[junit4] 2> ago 13, 2020 3:37:55 DA TARDE com.carrotsearch.randomizedtesting.RandomizedRunner$QueueUncaughtExceptionsHandler uncaughtException
[junit4] 2> WARNING: Uncaught exception in thread: Thread[Thread-0,5,TGRP-TestIndexWriter]
[junit4] 2> java.lang.AssertionError: java.lang.IllegalStateException: this writer hit an unrecoverable error; cannot commit
[junit4] 2> at __randomizedtesting.SeedInfo.seed([432EB011B0898067]:0)
[junit4] 2> at org.apache.lucene.index.TestIndexWriter.lambda$testRandomOperations$48(TestIndexWriter.java:3886)
[junit4] 2> at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] 2> Caused by: java.lang.IllegalStateException: this writer hit an unrecoverable error; cannot commit
[junit4] 2> at org.apache.lucene.index.IndexWriter.startCommit(IndexWriter.java:4930)
[junit4] 2> at org.apache.lucene.index.IndexWriter.prepareCommitInternal(IndexWriter.java:3365)
[junit4] 2> at org.apache.lucene.index.IndexWriter.commitInternal(IndexWriter.java:3664)
[junit4] 2> at org.apache.lucene.index.IndexWriter.commit(IndexWriter.java:3622)
[junit4] 2> at org.apache.lucene.index.TestIndexWriter.lambda$testRandomOperations$48(TestIndexWriter.java:3879)
[junit4] 2> ... 1 more
[junit4] 2> Suppressed: org.apache.lucene.store.AlreadyClosedException: refusing to delete any files: this IndexWriter hit an unrecoverable exception
[junit4] 2> at org.apache.lucene.index.IndexFileDeleter.ensureOpen(IndexFileDeleter.java:349)
[junit4] 2> at org.apache.lucene.index.IndexFileDeleter.deleteFiles(IndexFileDeleter.java:669)
[junit4] 2> at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:589)
[junit4] 2> at org.apache.lucene.index.IndexWriter.prepareCommitInternal(IndexWriter.java:3375)
[junit4] 2> ... 4 more
[junit4] 2> Caused by: org.apache.lucene.index.CorruptIndexException: Problem reading index from MockDirectoryWrapper(ByteBuffersDirectory@ebddcb6 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@2bd239b2) (resource=MockDir\
ectoryWrapper(ByteBuffersDirectory@ebddcb6 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@2bd239b2))
[junit4] 2> at org.apache.lucene.index.SegmentCoreReaders.<init>(SegmentCoreReaders.java:142)
[junit4] 2> at org.apache.lucene.index.SegmentReader.<init>(SegmentReader.java:83)
[junit4] 2> at org.apache.lucene.index.ReadersAndUpdates.getReader(ReadersAndUpdates.java:171)
[junit4] 2> at org.apache.lucene.index.ReadersAndUpdates.getReadOnlyClone(ReadersAndUpdates.java:213)
[junit4] 2> at org.apache.lucene.index.IndexWriter.lambda$getReader$0(IndexWriter.java:568)
[junit4] 2> at org.apache.lucene.index.IndexWriter.lambda$getReader$1(IndexWriter.java:614)
[junit4] 2> at org.apache.lucene.index.IndexWriter$2.onMergeComplete(IndexWriter.java:3461)
[junit4] 2> at org.apache.lucene.index.IndexWriter.commitMerge(IndexWriter.java:4078)
[junit4] 2> at org.apache.lucene.index.IndexWriter.mergeMiddle(IndexWriter.java:4697)
[junit4] 2> at org.apache.lucene.index.IndexWriter.merge(IndexWriter.java:4257)
[junit4] 2> at org.apache.lucene.index.IndexWriter$IndexWriterMergeSource.merge(IndexWriter.java:5808)
[junit4] 2> at org.apache.lucene.index.ConcurrentMergeScheduler.doMerge(ConcurrentMergeScheduler.java:624)
[junit4] 2> at org.apache.lucene.index.ConcurrentMergeScheduler$MergeThread.run(ConcurrentMergeScheduler.java:682)
[junit4] 2> Caused by: java.io.FileNotFoundException: _1m.fnm in dir=ByteBuffersDirectory@ebddcb6 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@2bd239b2
[junit4] 2> at org.apache.lucene.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:748)
[junit4] 2> at org.apache.lucene.store.Directory.openChecksumInput(Directory.java:157)
[junit4] 2> at org.apache.lucene.store.MockDirectoryWrapper.openChecksumInput(MockDirectoryWrapper.java:1044)
[junit4] 2> at org.apache.lucene.codecs.lucene60.Lucene60FieldInfosFormat.read(Lucene60FieldInfosFormat.java:113)
[junit4] 2> at org.apache.lucene.index.SegmentCoreReaders.<init>(SegmentCoreReaders.java:109)
[junit4] 2> ... 12 more
```
This one does reproduce with this PR (but not on mainline).
BTW, we need to fix mainline's `Reproduce with: ...` line to show `gradle` not `ant`?
This one also repros, only with this PR, but is maybe a test issue? Not sure:
```
[junit4] Started J0 PID(1331595@localhost).
[junit4] Suite: org.apache.lucene.index.TestIndexWriter
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestIndexWriter -Dtests.method=testSoftAndHardLiveDocs -Dtests.seed=8ECCF6879557EA9E -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=th-TH -Dtests.timezone=America/Shiprock -Dtests\
.asserts=true -Dtests.file.encoding=UTF-8
[junit4] ERROR 0.30s | TestIndexWriter.testSoftAndHardLiveDocs <<<
[junit4] > Throwable #1: org.apache.lucene.store.AlreadyClosedException: this IndexReader is closed
[junit4] > at __randomizedtesting.SeedInfo.seed([8ECCF6879557EA9E:5BCA4864AD808EEB]:0)
[junit4] > at org.apache.lucene.index.IndexReader.ensureOpen(IndexReader.java:257)
[junit4] > at org.apache.lucene.index.IndexReader.incRef(IndexReader.java:184)
[junit4] > at org.apache.lucene.index.IndexWriter.lambda$getReader$2(IndexWriter.java:653)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.open(StandardDirectoryReader.java:105)
[junit4] > at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:642)
[junit4] > at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:103)
[junit4] > at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:79)
[junit4] > at org.apache.lucene.index.TestIndexWriter.assertHardLiveDocs(TestIndexWriter.java:3638)
[junit4] > at org.apache.lucene.index.TestIndexWriter.testSoftAndHardLiveDocs(TestIndexWriter.java:3624)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene86): {id=PostingsFormat(name=LuceneFixedGap)}, docValues:{soft_delete=DocValuesFormat(name=Asserting)}, maxPointsInLeafNode=362, maxMBSortInHeap=5.242399601460019, sim=Asserting(RandomSi\
milarity(queryNorm=true): {}), locale=th-TH, timezone=America/Shiprock
[junit4] 2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=455736688,total=536870912
[junit4] 2> NOTE: All tests run in this JVM: [TestIndexWriter]
[junit4] Completed [1/1 (1!)] in 0.49s, 1 test, 1 error <<< FAILURES!
[junit4]
[junit4]
[junit4] Tests with failures [seed: 8ECCF6879557EA9E]:
[junit4] - org.apache.lucene.index.TestIndexWriter.testSoftAndHardLiveDocs
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674270585
256 iterations and no failures yet!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471146828
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
Review comment:
I added one new method. I don't think we should extract more WDYT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dnhatn commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
dnhatn commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472332671
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3179,9 +3317,9 @@ private long prepareCommitInternal() throws IOException {
SegmentInfos toCommit = null;
boolean anyChanges = false;
long seqNo;
- MergePolicy.MergeSpecification onCommitMerges = null;
- AtomicBoolean includeInCommit = new AtomicBoolean(true);
- final long maxCommitMergeWaitMillis = config.getMaxCommitMergeWaitMillis();
+ MergePolicy.MergeSpecification pointInTimeMerges = null;
+ AtomicBoolean hasTimedOut = new AtomicBoolean(false);
Review comment:
Should we rename this to stopCollectingMergedReaders? I find that name better.
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean stopCollectingMergedReaders = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
Review comment:
Should we keep track the clones iff `maxFullFlushMergeWaitMillis` is positive? I know this is quite trivial.
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean stopCollectingMergedReaders = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
Review comment:
Should we keep track the clones iff `maxFullFlushMergeWaitMillis` is positive? I know this is not expensive.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472336456
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3179,9 +3317,9 @@ private long prepareCommitInternal() throws IOException {
SegmentInfos toCommit = null;
boolean anyChanges = false;
long seqNo;
- MergePolicy.MergeSpecification onCommitMerges = null;
- AtomicBoolean includeInCommit = new AtomicBoolean(true);
- final long maxCommitMergeWaitMillis = config.getMaxCommitMergeWaitMillis();
+ MergePolicy.MergeSpecification pointInTimeMerges = null;
+ AtomicBoolean hasTimedOut = new AtomicBoolean(false);
Review comment:
++ I change it to a better name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472314458
##########
File path: lucene/core/src/java/org/apache/lucene/index/ReaderPool.java
##########
@@ -404,7 +404,7 @@ private PendingDeletes newPendingDeletes(SegmentReader reader, SegmentCommitInfo
private boolean noDups() {
Set<String> seen = new HashSet<>();
for(SegmentCommitInfo info : readerMap.keySet()) {
- assert !seen.contains(info.info.name);
+ assert !seen.contains(info.info.name) : "seen twice: " + info.info.name ;
Review comment:
many fun issues in this PR to be honest. IW is tricky as hell in some places like we are incRefing files in StandardDirectoryReader but not in IW for NRT readers is mindblowing :D
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472154682
##########
File path: lucene/core/src/java/org/apache/lucene/index/ReaderPool.java
##########
@@ -404,7 +404,7 @@ private PendingDeletes newPendingDeletes(SegmentReader reader, SegmentCommitInfo
private boolean noDups() {
Set<String> seen = new HashSet<>();
for(SegmentCommitInfo info : readerMap.keySet()) {
- assert !seen.contains(info.info.name);
+ assert !seen.contains(info.info.name) : "seen twice: " + info.info.name ;
Review comment:
I love seeing diffs like this one, adding a `String` message to an otherwise cryptic `assert`! It makes me realize you must have had a hellacious debugging session!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674118327
> @mikemccand nevermind I think I found the issue. I think it's ready now.
Yay! I will kick off new beasting (of all Lucene core + module tests) on my 128 core box!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-651166475
Here's a fun tragic failure test that repros:
```
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestIndexWriterOnVMError -Dtests.method=testUnknownError -Dtests.seed=BBEAF0EBC40AB8F7 -Dtests.slow=true -Dtests.badapples=true \
-Dtests.locale=brx -Dtests.timezone=Indian/Chagos -Dtests.asserts=true -Dtests.file.encoding=UTF-8
[junit4] FAILURE 0.30s | TestIndexWriterOnVMError.testUnknownError <<<
[junit4] > Throwable #1: java.lang.AssertionError
[junit4] > at __randomizedtesting.SeedInfo.seed([BBEAF0EBC40AB8F7:50BC2F6BC68D2EC7]:0)
[junit4] > at org.apache.lucene.index.IndexWriter.maybeCloseOnTragicEvent(IndexWriter.java:5026)
[junit4] > at org.apache.lucene.index.IndexWriter.tragicEvent(IndexWriter.java:5019)
[junit4] > at org.apache.lucene.index.IndexWriter.merge(IndexWriter.java:4214)
[junit4] > at org.apache.lucene.index.IndexWriter$IndexWriterMergeSource.merge(IndexWriter.java:5735)
[junit4] > at org.apache.lucene.index.SerialMergeScheduler.merge(SerialMergeScheduler.java:40)
[junit4] > at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:581)
[junit4] > at org.apache.lucene.index.DirectoryReader.open(DirectoryReader.java:103)
[junit4] > at org.apache.lucene.index.TestIndexWriterOnVMError.doTest(TestIndexWriterOnVMError.java:175)
[junit4] > at org.apache.lucene.index.TestIndexWriterOnVMError.testUnknownError(TestIndexWriterOnVMError.java:251)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene86): {text_payloads=PostingsFormat(name=Direct), text_vectors=TestBloomFilteredLucenePostings(BloomFilteringPostingsFormat(Luc\
ene84)), text1=TestBloomFilteredLucenePostings(BloomFilteringPostingsFormat(Lucene84)), id=PostingsFormat(name=Direct)}, docValues:{dv3=DocValuesFormat(name=Asserting), dv2=DocValuesFormat\
(name=Lucene80), dv5=DocValuesFormat(name=Asserting), dv=DocValuesFormat(name=Lucene80), dv4=DocValuesFormat(name=Lucene80)}, maxPointsInLeafNode=1228, maxMBSortInHeap=6.874571632539512, s\
im=Asserting(RandomSimilarity(queryNorm=true): {text_payloads=IB SPL-L1, text_vectors=BM25(k1=1.2,b=0.75), text1=DFI(Saturated)}), locale=brx, timezone=Indian/Chagos
[junit4] 2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=522930136,total=536870912
[junit4] 2> NOTE: All tests run in this JVM: [TestIndexWriterOnVMError]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472162610
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
OK let's leave this be for now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msokolov commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
msokolov commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-651116085
So this would merge small commits on refresh? That's cool. I wonder if it would be more obvious to users if we call the MergeTrigger REFRESH? I see that the trigger method is IndexWriter.getReader, but it seems like ultimately the higher level event that is more familiar is refresh.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471145995
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
we actually never abort these merges. there is no reason why we should do that. we might have done most of the work, we only abort merges if we shutdown our writers. I hope that makes sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-653203756
this assertion checks that we don't hold the full flush lock. I am trying to remember why I added this assertion. I understand the assertion above which also has a comment but I am unsure about the full flush lock it should not cause any deadlocks or any issues. I will keep thinking about it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-677710206
I will restart beasting on the latest PR now. Thanks @s1monw!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-671301636
@mikemccand I do understand the issue now why holding the _flushLock_ is illegal here. The problem is again the lock ordering in combination with the _commitLock_. One option that we have here is to remove the _flushLock_ altogether and replace it's usage with the _commitLock_. I guess we need to find a better or new name for it but I don't see where having two different locks buys us much since they are both really just used to sync on administration of the IW. I personally also don't see why it would buys us anything in terms of concurrency. WDYT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674031356
I pushed a new commit and fixed `ant test -Dtestcase=TestIndexWriter -Dtests.method=testRandomOperations -Dtests.seed=432EB011B0898067 -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=pt-ST -Dtests.timezone=US/Mountain -Dtests.asserts=true -Dtest\
s.file.encoding=UTF-8`
yet I can't reproduce this one: `reproduce with: ant test -Dtestcase=TestIndexWriter -Dtests.method=testSoftAndHardLiveDocs -Dtests.seed=8ECCF6879557EA9E -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=th-TH -Dtests.timezone=America/Shiprock -Dtests\
.asserts=true -Dtests.file.encoding=UTF-8` does it reproduce for you on this branch with the latest commit?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-678796020
> @mikemccand I think we are ready here WDYT?
+1! Thanks @s1monw!
I will continue beasting. The failures are very rare now ... I ran 196 iters of slow + nightly Lucene core + modules tests, and hit only ~4 interesting failures.
E.g. this failing seed repros on the PR but not on mainline:
```
org.apache.lucene.index.TestIndexWriterOnVMError > testUnknownError FAILED
org.apache.lucene.index.CorruptIndexException: Unexpected file read error while reading index. (resource=BufferedChecksumIndexInput(MockIndexInputWrapper((clone of) ByteBuffersIndexInput (file=pending_segments_2, buffers\
=258 bytes, block size: 1, blocks: 1, position: 0))))
at __randomizedtesting.SeedInfo.seed([587A104EFE0C57E1:B32CCFCEFC8BC1D1]:0)
at org.apache.lucene.index.SegmentInfos.readCommit(SegmentInfos.java:300)
at org.apache.lucene.index.CheckIndex.checkIndex(CheckIndex.java:521)
at org.apache.lucene.util.TestUtil.checkIndex(TestUtil.java:301)
at org.apache.lucene.store.MockDirectoryWrapper.close(MockDirectoryWrapper.java:836)
at org.apache.lucene.index.TestIndexWriterOnVMError.doTest(TestIndexWriterOnVMError.java:89)
at org.apache.lucene.index.TestIndexWriterOnVMError.testUnknownError(TestIndexWriterOnVMError.java:251)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1754)
at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:942)
at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:978)
at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:992)
at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:49)
at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:48)
at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:819)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:470)
at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:951)
at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:836)
at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:887)
at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:898)
at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:41)
at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:54)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:826)
at java.base/java.lang.Thread.run(Thread.java:834)
Caused by:
java.io.FileNotFoundException: _0.si in dir=ByteBuffersDirectory@1bae3fe1 lockFactory=org.apache.lucene.store.SingleInstanceLockFactory@38275f41
at org.apache.lucene.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:748)
at org.apache.lucene.store.Directory.openChecksumInput(Directory.java:157)
at org.apache.lucene.store.MockDirectoryWrapper.openChecksumInput(MockDirectoryWrapper.java:1044)
at org.apache.lucene.codecs.lucene86.Lucene86SegmentInfoFormat.read(Lucene86SegmentInfoFormat.java:91)
at org.apache.lucene.index.SegmentInfos.readCommit(SegmentInfos.java:364)
at org.apache.lucene.index.SegmentInfos.readCommit(SegmentInfos.java:298)
... 41 more
....
2> NOTE: reproduce with: ant test -Dtestcase=TestIndexWriterOnVMError -Dtests.method=testUnknownError -Dtests.seed=587A104EFE0C57E1 -Dtests.nightly=true -Dtests.slow=true -Dtests.badapples=true -Dtests.linedocsfile=/l/sim\
on/lucene/test-framework/src/resources/org/apache/lucene/util/2000mb.txt.gz -Dtests.locale=zh-CN -Dtests.timezone=SystemV/MST7MDT -Dtests.asserts=true -Dtests.file.encoding=UTF-8
2> NOTE: leaving temporary files on disk at: /l/simon/lucene/core/build/tmp/tests-tmp/lucene.index.TestIndexWriterOnVMError_587A104EFE0C57E1-003
2> NOTE: test params are: codec=Asserting(Lucene86): {text_payloads=BlockTreeOrds(blocksize=128), text_vectors=PostingsFormat(name=Asserting), text1=PostingsFormat(name=Asserting), id=BlockTreeOrds(blocksize=128)}, docValu\
es:{dv3=DocValuesFormat(name=Lucene80), dv2=DocValuesFormat(name=Asserting), dv5=DocValuesFormat(name=Lucene80), dv=DocValuesFormat(name=Asserting), dv4=DocValuesFormat(name=Asserting)}, maxPointsInLeafNode=696, maxMBSortInH\
eap=6.040673619645681, sim=Asserting(RandomSimilarity(queryNorm=false): {text_payloads=IB SPL-DZ(0.3), text_vectors=DFR I(ne)L3(800.0), text1=org.apache.lucene.search.similarities.BooleanSimilarity@6f4329a1}), locale=zh-CN, \
timezone=SystemV/MST7MDT
2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=241525696,total=268435456
2> NOTE: All tests run in this JVM: [TestIndexWriterOnVMError]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-650640435
Wow! This is incredibly simple! Thanks to the clean approach @s1monw worked out for commit-on-merge, awesome! I think tests would exercise this due to `MockRandomMergePolicy`? I'll try beasting :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] msfroh commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
msfroh commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r470274067
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3321,8 +3395,11 @@ private long prepareCommitInternal() throws IOException {
* below. We also ensure that we pull the merge readers while holding {@code IndexWriter}'s lock. Otherwise
* we could see concurrent deletions/updates applied that do not belong to the segment.
Review comment:
This Javadoc will need to be updated to reflect the broader use of this method.
Also, is `preparePointInTimeMerge` (without the `on`) a better name?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-679088291
> @mikemccand this test reproduces on master too I opened https://issues.apache.org/jira/browse/LUCENE-9477
Great, thanks @s1monw! Given that beasting is now uncovering pre-existing issues I think we should push this PR! Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674391203
919 iterations and only this failure!
```
1> TEST: now get reader
> java.io.IOException: MockDirectoryWrapper: file "_3.cfs" is still open: cannot delete
> at __randomizedtesting.SeedInfo.seed([9EEEDD0028F78B57:7E45B4FC8975A53E]:0)
> at org.apache.lucene.store.MockDirectoryWrapper.deleteFile(MockDirectoryWrapper.java:600)
> at org.apache.lucene.store.LockValidatingDirectoryWrapper.deleteFile(LockValidatingDirectoryWrapper.java:38)
> at org.apache.lucene.index.IndexFileDeleter.deleteFile(IndexFileDeleter.java:696)
> at org.apache.lucene.index.IndexFileDeleter.deleteFiles(IndexFileDeleter.java:690)
> at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:589)
> at org.apache.lucene.index.IndexFileDeleter.checkpoint(IndexFileDeleter.java:531)
> at org.apache.lucene.index.IndexWriter.checkpoint(IndexWriter.java:2625)
> at org.apache.lucene.index.IndexWriter.commitMerge(IndexWriter.java:4201)
> at org.apache.lucene.index.IndexWriter.mergeMiddle(IndexWriter.java:4818)
> at org.apache.lucene.index.IndexWriter.merge(IndexWriter.java:4265)
> at org.apache.lucene.index.IndexWriter$IndexWriterMergeSource.merge(IndexWriter.java:5816)
> at org.apache.lucene.index.SerialMergeScheduler.merge(SerialMergeScheduler.java:40)
> at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:639)
> at org.apache.lucene.index.IndexWriter.getReader(IndexWriter.java:471)
> at org.apache.lucene.index.TestIndexWriterReader.doTestIndexWriterReopenSegment(TestIndexWriterReader.java:573)
> at org.apache.lucene.index.TestIndexWriterReader.testIndexWriterReopenSegmentFullMerge(TestIndexWriterReader.java:523)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1754)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:942)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:978)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:992)
> at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:49)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
> at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:48)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:819)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:470)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:951)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:836)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:887)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:898)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:41)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
> at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:54)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:826)
> at java.base/java.lang.Thread.run(Thread.java:834)
>
> Caused by:
> java.lang.RuntimeException: unclosed IndexInput: _3.cfs
> at org.apache.lucene.store.MockDirectoryWrapper.addFileHandle(MockDirectoryWrapper.java:730)
> at org.apache.lucene.store.MockDirectoryWrapper.openInput(MockDirectoryWrapper.java:773)
> at org.apache.lucene.codecs.lucene50.Lucene50CompoundReader.<init>(Lucene50CompoundReader.java:77)
> at org.apache.lucene.codecs.lucene50.Lucene50CompoundFormat.getCompoundReader(Lucene50CompoundFormat.java:71)
> at org.apache.lucene.index.SegmentCoreReaders.<init>(SegmentCoreReaders.java:101)
> at org.apache.lucene.index.SegmentReader.<init>(SegmentReader.java:83)
> at org.apache.lucene.index.ReadersAndUpdates.getReader(ReadersAndUpdates.java:171)
> at org.apache.lucene.index.ReadersAndUpdates.getReadOnlyClone(ReadersAndUpdates.java:213)
> at org.apache.lucene.index.IndexWriter.lambda$getReader$0(IndexWriter.java:568)
> at org.apache.lucene.index.IndexWriter.lambda$getReader$1(IndexWriter.java:618)
> at org.apache.lucene.index.IndexWriter$2.onMergeComplete(IndexWriter.java:3467)
> at org.apache.lucene.index.IndexWriter.commitMerge(IndexWriter.java:4086)
> ... 44 more
2> NOTE: reproduce with: ant test -Dtestcase=TestIndexWriterReader -Dtests.method=testIndexWriterReopenSegmentFullMerge -Dtests.seed=9EEEDD0028F78B57 -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=ckb -Dtests.timezone=Asia/Kabul -Dtests\
.asserts=true -Dtests.file.encoding=UTF-8
2> NOTE: leaving temporary files on disk at: /l/simon/lucene/core/build/tmp/tests-tmp/lucene.index.TestIndexWriterReader_9EEEDD0028F78B57-001
2> NOTE: test params are: codec=Asserting(Lucene86), sim=Asserting(RandomSimilarity(queryNorm=true): {field1=IB SPL-DZ(0.3), field=DFR GLZ(0.3), indexname=DFR I(n)LZ(0.3), field3=org.apache.lucene.search.similarities.BooleanSimilarity@156f33cc, f\
ield2=IB LL-D3(800.0), field5=DFI(Standardized), field4=IB SPL-D2}), locale=ckb, timezone=Asia/Kabul
2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=222958128,total=268435456
2> NOTE: All tests run in this JVM: [TestPolygon, TestLucene80NormsFormatMergeInstance, TestLucene86SegmentInfoFormat, TestIndexWriterUnicode, TestSoftDeletesRetentionMergePolicy, TestLogMergePolicy, TestPayloads, TestIndexWriter, TestIndexingSeq\
uenceNumbers, TestCustomNorms, TestIndexWriterReader]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dnhatn commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
dnhatn commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-675222698
@s1monw Thank you so much for the explanation. I wrote a test that sneaks some updates during the merge. And the returned reader has duplicates with my suggestion, but not with the current approach. I will review the PR tomorrow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-675459999
Those were the only two failures found after 1033 test iterations!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-673088302
@mikemccand @msokolov @msfroh I pushed a new and slightly more complex but afaik correct approach to do the merge during getReader. Would be great to get some feedback. I think it's still pretty contained.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-679034737
@mikemccand this test reproduces on master too I opened https://issues.apache.org/jira/browse/LUCENE-9477
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-675571884
@mikemccand I pushed a fix for the failures. I will add a dedicated test to make sure it's covered too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-650638907
@mikemccand @msokolov @msfroh I tried to build the minimal necessary code to enable this feature. I do actually think that the refresh / getReader change is much simpler than the commit part since we don't need to keep a cloned SegmentInfos in sync. I opened this really as a question if it can be that simple?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471957713
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -3335,49 +3416,60 @@ public void mergeFinished(boolean committed, boolean segmentDropped) throws IOEx
// includedInCommit will be set (above, by our caller) to false if the allowed max wall clock
// time (IWC.getMaxCommitMergeWaitMillis()) has elapsed, which means we did not make the timeout
// and will not commit our merge to the to-be-commited SegmentInfos
-
if (segmentDropped == false
&& committed
- && includeInCommit.get()) {
+ && includeMergeResult.get()) {
+
+ // make sure onMergeComplete really was called:
+ assert origInfo != null;
if (infoStream.isEnabled("IW")) {
infoStream.message("IW", "now apply merge during commit: " + toWrap.segString());
}
- // make sure onMergeComplete really was called:
- assert origInfo != null;
-
- deleter.incRef(origInfo.files());
+ if (trigger == MergeTrigger.COMMIT) { // if we do this in a getReader call here this is obsolete
+ deleter.incRef(origInfo.files());
Review comment:
I extended the comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471960276
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -630,6 +694,64 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
return r;
}
+ private StandardDirectoryReader finishGetReaderMerge(AtomicBoolean hasTimedOut, Map<String, SegmentReader> mergedReaders,
+ Map<String, SegmentReader> openedReadOnlyClones, SegmentInfos openingSegmentInfos,
+ boolean applyAllDeletes, boolean writeAllDeletes,
+ MergePolicy.MergeSpecification onCommitMerges, long maxCommitMergeWaitMillis) throws IOException {
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
+ assert openingSegmentInfos != null;
+ synchronized (this) {
+ hasTimedOut.set(true);
Review comment:
I renamed it to `stopCollectingMergedReaders`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-678796112
Hmm, not that I am using the larger (`2000mb`) line docs file, and that might be needed to provoke failure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674564541
@mikemccand I pushed a fix. it's just violating a test condition IMO please double check
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-673727019
Ugh, also these test failures that I put onto the wrong PR: https://github.com/apache/lucene-solr/pull/1743#issuecomment-673726633
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-651156942
> So this would merge small commits on refresh?
Small segments, yes.
> I wonder if it would be more obvious to users if we call the MergeTrigger REFRESH?
+1, refresh is more recognized in the outside world :)
I have been beasting this and uncovering small test failures, in tests that are confused that they do not have the expected number of segments. I'll push some fixes for those ...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r472337839
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -545,18 +546,54 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
// obtained during this flush are pooled, the first time
// this method is called:
readerPool.enableReaderPooling();
- DirectoryReader r = null;
+ StandardDirectoryReader r = null;
doBeforeFlush();
- boolean anyChanges = false;
+ boolean anyChanges;
/*
* for releasing a NRT reader we must ensure that
* DW doesn't add any segments or deletes until we are
* done with creating the NRT DirectoryReader.
* We release the two stage full flush after we are done opening the
* directory reader!
*/
+ MergePolicy.MergeSpecification onGetReaderMerges = null;
+ AtomicBoolean stopCollectingMergedReaders = new AtomicBoolean(false);
+ Map<String, SegmentReader> mergedReaders = new HashMap<>();
+ Map<String, SegmentReader> openedReadOnlyClones = new HashMap<>();
+ // this function is used to control which SR are opened in order to keep track of them
+ // and to reuse them in the case we wait for merges in this getReader call.
+ IOUtils.IOFunction<SegmentCommitInfo, SegmentReader> readerFactory = sci -> {
+ final ReadersAndUpdates rld = getPooledInstance(sci, true);
+ try {
+ assert Thread.holdsLock(IndexWriter.this);
+ SegmentReader segmentReader = rld.getReadOnlyClone(IOContext.READ);
+ openedReadOnlyClones.put(sci.info.name, segmentReader);
Review comment:
++
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-673112665
Awesome, thanks @s1monw! I will try to have a look soon. I kicked off beasting of all Lucene (core + modules) tests with this change ... no failures yet after 31 iterations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674091972
@mikemccand nevermind I think I found the issue. I think it's ready now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-675458466
Beasting ran all night (1031 iterations) and uncovered this failure, which reproduces with this PR but not on mainline:
```
[mkdir] Created dir: /l/simon/lucene/build/core/test
[junit4:pickseed] Seed property 'tests.seed' already defined: 313A1CF00C235D4F
[mkdir] Created dir: /l/simon/lucene/build/core/test/temp
[mkdir] Created dir: /l/simon/.caches/test-stats/core
[junit4] <JUnit4> says hi! Master seed: 313A1CF00C235D4F
[junit4] Executing 1 suite with 1 JVM.
[junit4]
[junit4] Started J0 PID(972126@localhost).
[junit4] Suite: org.apache.lucene.codecs.perfield.TestPerFieldDocValuesFormat
[junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestPerFieldDocValuesFormat -Dtests.method=testSparseBinaryVariableLengthVsStoredFields -Dtests.seed=313A1CF00C235D4F -Dtests.slow=true -Dtest\
s.badapples=true -Dtests.locale=ur -Dtests.timezone=Mexico/BajaNorte -Dtests.asserts=true -Dtests.file.encoding=UTF-8
[junit4] ERROR 0.43s | TestPerFieldDocValuesFormat.testSparseBinaryVariableLengthVsStoredFields <<<
[junit4] > Throwable #1: java.nio.file.NoSuchFileException: _j.si
[junit4] > at __randomizedtesting.SeedInfo.seed([313A1CF00C235D4F:5C414B294A4A9C4D]:0)
[junit4] > at org.apache.lucene.store.ByteBuffersDirectory.deleteFile(ByteBuffersDirectory.java:148)
[junit4] > at org.apache.lucene.store.MockDirectoryWrapper.deleteFile(MockDirectoryWrapper.java:607)
[junit4] > at org.apache.lucene.store.LockValidatingDirectoryWrapper.deleteFile(LockValidatingDirectoryWrapper.java:38)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.deleteFile(IndexFileDeleter.java:696)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.deleteFiles(IndexFileDeleter.java:690)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:589)
[junit4] > at org.apache.lucene.index.IndexFileDeleter.decRef(IndexFileDeleter.java:620)
[junit4] > at org.apache.lucene.index.IndexWriter.decRefDeleter(IndexWriter.java:5354)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.lambda$doClose$1(StandardDirectoryReader.java:370)
[junit4] > at org.apache.lucene.index.StandardDirectoryReader.doClose(StandardDirectoryReader.java:384)
[junit4] > at org.apache.lucene.index.IndexReader.decRef(IndexReader.java:244)
[junit4] > at org.apache.lucene.index.IndexReader.close(IndexReader.java:385)
[junit4] > at org.apache.lucene.index.BaseDocValuesFormatTestCase.doTestBinaryVsStoredFields(BaseDocValuesFormatTestCase.java:1527)
[junit4] > at org.apache.lucene.index.BaseDocValuesFormatTestCase.doTestBinaryVariableLengthVsStoredFields(BaseDocValuesFormatTestCase.java:1585)
[junit4] > at org.apache.lucene.index.BaseDocValuesFormatTestCase.testSparseBinaryVariableLengthVsStoredFields(BaseDocValuesFormatTestCase.java:1579)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[junit4] > at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[junit4] > at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[junit4] > at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[junit4] > at java.base/java.lang.Thread.run(Thread.java:834)
[junit4] 2> NOTE: test params are: codec=Asserting(Lucene86): {}, docValues:{}, maxPointsInLeafNode=1657, maxMBSortInHeap=6.829528977246159, sim=Asserting(RandomSimilarity(queryNorm=true): {}), loc\
ale=ur, timezone=Mexico/BajaNorte
[junit4] 2> NOTE: Linux 5.5.6-arch1-1 amd64/Oracle Corporation 11.0.6 (64-bit)/cpus=128,threads=1,free=516818968,total=536870912
[junit4] 2> NOTE: All tests run in this JVM: [TestPerFieldDocValuesFormat]
[junit4] Completed [1/1 (1!)] in 0.62s, 1 test, 1 error <<< FAILURES!
[junit4]
[junit4]
[junit4] Tests with failures [seed: 313A1CF00C235D4F]:
[junit4] - org.apache.lucene.codecs.perfield.TestPerFieldDocValuesFormat.testSparseBinaryVariableLengthVsStoredFields
[junit4]
[junit4]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on a change in pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw commented on a change in pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#discussion_r471991665
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
##########
@@ -607,6 +633,57 @@ DirectoryReader getReader(boolean applyAllDeletes, boolean writeAllDeletes) thro
}
}
}
+ if (onCommitMerges != null) { // only relevant if we do merge on getReader
+ boolean replaceReaderSuccess = false;
+ try {
+ mergeScheduler.merge(mergeSource, MergeTrigger.GET_READER);
+ onCommitMerges.await(maxCommitMergeWaitMillis, TimeUnit.MILLISECONDS);
Review comment:
I'd not want to add another option to IWC unless absolutely necessary. Maybe we can just keep one for now and if somebody has a good usecase we can still add? I think we have the ability to disable it entirely for one or the other trigger which should be enough in most cases?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw edited a comment on pull request #1623: LUCENE-8962: Merge segments on getReader
Posted by GitBox <gi...@apache.org>.
s1monw edited a comment on pull request #1623:
URL: https://github.com/apache/lucene-solr/pull/1623#issuecomment-674718362
> @s1monw I like your first commit :). I wonder if we can continue that approach by moving the merge logic outside fullFlushLock and reopen a new reader if the merge completes within the timeout? But, I might be missing something obvious here.
hey @dnhatn I wish we could! I first wrote a longish reply to your comment but then decided to push some documentation on how getReader works and why we can't do things outside of the lock, or at least not easily. Lemme know if it helps.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org