jimczi commented on PR #16581:
URL: https://github.com/apache/lucene/pull/16581#issuecomment-5521200201
Thanks Houston, this all looks good now. The per-type split in
`addDiskDiffToPacket` is clean and the sorted-numeric carry-over reads right: a
still-multi-valued doc can't have been updated, so a single or absent value
diffed against the base is exactly the set of updates that flushed during the
merge.
Two small things.
The carry-over fix itself doesn't have a test yet. The new multi-valued
tests cover the fold and dense rewrite, but nothing resolves an update *while a
merge is running*, so `addSortedNumericDiskDiffToPacket` never actually runs. I
wrote one on top of `TestMergeCarryOverFromDisk` (from #16570, it already has
the paused-merge machinery) and it passes 30 iters. Feel free to grab it, just
add `import org.apache.lucene.document.SortedNumericDocValuesField;`:
```java
/**
* A single-valued sorted-numeric update that resolves onto a segment
while it is being merged must
* be carried over, and an untouched multi-valued doc must keep its whole
value set. Exercises the
* sorted-numeric branch of the disk carry-over
(addSortedNumericDiskDiffToPacket).
*/
public void testSortedNumericUpdateResolvedDuringMergeIsCarriedOver()
throws Exception {
MergePausingDirectory dir = new MergePausingDirectory(newDirectory());
IndexWriterConfig conf =
newIndexWriterConfig(new MockAnalyzer(random()))
.setMergeScheduler(new ConcurrentMergeScheduler());
IndexWriter writer = new IndexWriter(dir, conf);
// First segment: doc 0 single-valued, doc 1 genuinely multi-valued
(never updated).
Document d0 = new Document();
d0.add(new StringField("id", "0", StringField.Store.NO));
d0.add(new SortedNumericDocValuesField("snv", 10));
writer.addDocument(d0);
Document d1 = new Document();
d1.add(new StringField("id", "1", StringField.Store.NO));
d1.add(new SortedNumericDocValuesField("snv", 20));
d1.add(new SortedNumericDocValuesField("snv", 21));
writer.addDocument(d1);
writer.commit();
// Second segment so the merge has two sources.
for (int i = 2; i < 6; i++) {
Document d = new Document();
d.add(new StringField("id", Integer.toString(i),
StringField.Store.NO));
d.add(new SortedNumericDocValuesField("snv", i * 10L));
writer.addDocument(d);
}
writer.commit();
Thread merger =
new Thread(
() -> {
try {
writer.forceMerge(1);
} catch (Throwable t) {
dir.failure.compareAndSet(null, t);
}
},
"forceMerge");
merger.start();
dir.mergeStarted.await();
// Update a single-valued doc in each merging segment, then resolve them
to disk.
writer.updateSortedNumericDocValue(new Term("id", "0"), "snv", 100);
writer.updateSortedNumericDocValue(new Term("id", "4"), "snv", 104);
try (DirectoryReader r = DirectoryReader.open(writer)) {
assertNotNull(r);
}
dir.resumeMerge.countDown();
merger.join();
assertNull("forceMerge failed: " + dir.failure.get(), dir.failure.get());
try (DirectoryReader reader = DirectoryReader.open(writer)) {
assertEquals(1, reader.leaves().size()); // single merged segment
LeafReader leaf = reader.leaves().get(0).reader();
SortedNumericDocValues snv = leaf.getSortedNumericDocValues("snv");
Terms idTerms = leaf.terms("id");
TermsEnum te = idTerms.iterator();
java.util.Map<String, long[]> byId = new java.util.HashMap<>();
BytesRef t;
while ((t = te.next()) != null) {
PostingsEnum pe = te.postings(null, PostingsEnum.NONE);
int docId = pe.nextDoc();
assertTrue(snv.advanceExact(docId));
long[] vals = new long[snv.docValueCount()];
for (int i = 0; i < vals.length; i++) {
vals[i] = snv.nextValue();
}
byId.put(t.utf8ToString(), vals);
}
// The two updates resolved during the merge were carried over.
assertArrayEquals("carried update for doc 0", new long[] {100},
byId.get("0"));
assertArrayEquals("carried update for doc 4", new long[] {104},
byId.get("4"));
// The untouched multi-valued doc kept its whole value set through the
merge.
assertArrayEquals("untouched multi-valued doc preserved", new long[]
{20, 21}, byId.get("1"));
// An untouched single-valued doc is unchanged.
assertArrayEquals(new long[] {30}, byId.get("3"));
}
writer.close();
dir.close();
}
```
Also the comments in `testDenseRewriteOverMultiValuedBase` and
`testFoldToDenseOverMultiValuedBase` still point at
`ReadersAndUpdates.MergedSortedNumericDocValues`, which is gone 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
