msokolov commented on code in PR #16401:
URL: https://github.com/apache/lucene/pull/16401#discussion_r3598299099
##########
lucene/core/src/test/org/apache/lucene/index/TestHnswMergeAbort.java:
##########
@@ -33,52 +36,71 @@
import org.apache.lucene.tests.util.TestUtil;
import org.apache.lucene.util.InfoStream;
import org.apache.lucene.util.NamedThreadFactory;
+import org.apache.lucene.util.ThreadInterruptedException;
/**
- * Tests that aborting a merge (e.g. via {@link IndexWriter#rollback()})
promptly interrupts HNSW
- * graph construction instead of blocking until the entire graph is built.
+ * Tests that aborting a merge (e.g. via {@link IndexWriter#rollback()})
interrupts HNSW graph
+ * construction instead of building the entire graph.
+ *
+ * <p>The merge thread is held at the start of the graph build until the merge
is marked aborted,
+ * and the test then asserts that the build never ran to completion, which
holds at any segment
+ * size. The abort exception is deliberately not asserted on: it is swallowed
as expected control
+ * flow, and a small unchecked build can finish without ever throwing.
*/
public class TestHnswMergeAbort extends LuceneTestCase {
private static final int DIM = 96;
private static final int SEGMENTS = 4;
- private static final int DOCS_PER_SEGMENT = 12_000;
- private static final int BEAM_WIDTH = 250;
+ private static final int DOCS_PER_SEGMENT = 1_000;
+ private static final int BEAM_WIDTH = 100;
+ // always build a graph, no matter how small the segment is
+ private static final int TINY_SEGMENTS_THRESHOLD = 0;
+ private static final int LIVE_DOCS_AFTER_DELETES = SEGMENTS *
DOCS_PER_SEGMENT / 2;
/**
* Every segment carries more than {@code
IncrementalHnswGraphMerger#DELETE_PCT_THRESHOLD}
* deletions, so no source graph is eligible as a base and the merged graph
is rebuilt from
* scratch via {@code HnswGraphBuilder#addVectors}.
*/
public void testRollbackDuringFullRebuildMerge() throws Exception {
Review Comment:
does this test rely on the fact that HNSW merging work is done on a
background thread? If we had single-threaded HNSW merging (as we used to do)
this would deadlock because the InfoStream would be waiting for the merging
operation to be aborted, but it could not progress because it *is* the merging
operation?
I guess that should be OK since all of our mergers do use their own thread
pools 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]
