Skip to content

Commit f7608f1

Browse files
committed
Check if merge is aborted during HNSW graph construction (#16367)
Graph construction is a pure-CPU loop that performs no writes, so it never observed OneMerge's abort flag: IndexWriter#rollback/abortMerges blocked until the entire graph was built (tens of minutes for large vector segments). Wire MergeState#checkAborted from Lucene99HnswVectorsWriter through the HNSW graph mergers into the builders, and invoke it before every node insertion in HnswGraphBuilder#addGraphNodeInternal - the single point that all insertion paths (full rebuild, graph-join, concurrent workers) funnel through. Flush-time graph builds are unaffected (no check set).
1 parent a9eb52b commit f7608f1

8 files changed

Lines changed: 222 additions & 6 deletions

File tree

lucene/CHANGES.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ Optimizations
178178

179179
Bug Fixes
180180
---------------------
181+
* GITHUB#16367: HNSW graph construction now periodically checks whether the surrounding merge has
182+
been aborted, so IndexWriter#rollback and abortMerges no longer block until the entire graph is
183+
built. (Jeho Jeong)
184+
181185
* GITHUB#14049: Randomize KNN codec params in RandomCodec. Fixes scalar quantization div-by-zero
182186
when all values are identical. (Mike Sokolov)
183187

lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99HnswVectorsWriter.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,8 @@ private void buildAndWriteGraph(
473473
mergeState.intraMergeTaskExecutor == null
474474
? null
475475
: new TaskExecutor(mergeState.intraMergeTaskExecutor),
476-
numMergeWorkers);
476+
numMergeWorkers,
477+
mergeState::checkAborted);
477478
for (int i = 0; i < mergeState.liveDocs.length; i++) {
478479
if (hasVectorValues(mergeState.fieldInfos[i], fieldInfo.name)) {
479480
merger.addReader(
@@ -626,10 +627,11 @@ private HnswGraphMerger createGraphMerger(
626627
FieldInfo fieldInfo,
627628
RandomVectorScorerSupplier scorerSupplier,
628629
TaskExecutor parallelMergeTaskExecutor,
629-
int numParallelMergeWorkers) {
630+
int numParallelMergeWorkers,
631+
IORunnable abortCheck) {
630632
if (mergeExec != null) {
631633
return new ConcurrentHnswMerger(
632-
fieldInfo, scorerSupplier, M, beamWidth, mergeExec, numMergeWorkers);
634+
fieldInfo, scorerSupplier, M, beamWidth, mergeExec, numMergeWorkers, abortCheck);
633635
}
634636
if (parallelMergeTaskExecutor != null && numParallelMergeWorkers > 1) {
635637
return new ConcurrentHnswMerger(
@@ -638,9 +640,10 @@ private HnswGraphMerger createGraphMerger(
638640
M,
639641
beamWidth,
640642
parallelMergeTaskExecutor,
641-
numParallelMergeWorkers);
643+
numParallelMergeWorkers,
644+
abortCheck);
642645
}
643-
return new IncrementalHnswGraphMerger(fieldInfo, scorerSupplier, M, beamWidth);
646+
return new IncrementalHnswGraphMerger(fieldInfo, scorerSupplier, M, beamWidth, abortCheck);
644647
}
645648

646649
@Override

lucene/core/src/java/org/apache/lucene/util/hnsw/ConcurrentHnswMerger.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.lucene.search.TaskExecutor;
3030
import org.apache.lucene.util.BitSet;
3131
import org.apache.lucene.util.FixedBitSet;
32+
import org.apache.lucene.util.IORunnable;
3233

3334
/** This merger merges graph in a concurrent manner, by using {@link HnswConcurrentMergeBuilder} */
3435
public class ConcurrentHnswMerger extends IncrementalHnswGraphMerger {
@@ -46,7 +47,24 @@ public ConcurrentHnswMerger(
4647
int beamWidth,
4748
TaskExecutor taskExecutor,
4849
int numWorker) {
49-
super(fieldInfo, scorerSupplier, M, beamWidth);
50+
this(fieldInfo, scorerSupplier, M, beamWidth, taskExecutor, numWorker, null);
51+
}
52+
53+
/**
54+
* @param fieldInfo FieldInfo for the field being merged
55+
* @param abortCheck optional check invoked before every node insertion during graph construction;
56+
* may throw {@link org.apache.lucene.index.MergePolicy.MergeAbortedException} to abort the
57+
* build when the surrounding merge has been aborted, or null
58+
*/
59+
public ConcurrentHnswMerger(
60+
FieldInfo fieldInfo,
61+
RandomVectorScorerSupplier scorerSupplier,
62+
int M,
63+
int beamWidth,
64+
TaskExecutor taskExecutor,
65+
int numWorker,
66+
IORunnable abortCheck) {
67+
super(fieldInfo, scorerSupplier, M, beamWidth, abortCheck);
5068
this.taskExecutor = taskExecutor;
5169
this.numWorker = numWorker;
5270
}

lucene/core/src/java/org/apache/lucene/util/hnsw/HnswBuilder.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.io.IOException;
2121
import org.apache.lucene.internal.hppc.IntHashSet;
22+
import org.apache.lucene.util.IORunnable;
2223
import org.apache.lucene.util.InfoStream;
2324

2425
/**
@@ -46,6 +47,14 @@ public interface HnswBuilder {
4647
/** Set info-stream to output debugging information */
4748
void setInfoStream(InfoStream infoStream);
4849

50+
/**
51+
* Sets a check that is invoked before every node insertion during graph construction. The check
52+
* may throw an exception to abort the build promptly, e.g. {@link
53+
* org.apache.lucene.index.MergePolicy.MergeAbortedException} when the merge that triggered the
54+
* build has been aborted.
55+
*/
56+
void setAbortCheck(IORunnable abortCheck);
57+
4958
OnHeapHnswGraph getGraph();
5059

5160
/**

lucene/core/src/java/org/apache/lucene/util/hnsw/HnswConcurrentMergeBuilder.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.lucene.search.TaskExecutor;
3232
import org.apache.lucene.util.BitSet;
3333
import org.apache.lucene.util.FixedBitSet;
34+
import org.apache.lucene.util.IORunnable;
3435
import org.apache.lucene.util.InfoStream;
3536

3637
/**
@@ -134,6 +135,13 @@ public void setInfoStream(InfoStream infoStream) {
134135
}
135136
}
136137

138+
@Override
139+
public void setAbortCheck(IORunnable abortCheck) {
140+
for (HnswBuilder worker : workers) {
141+
worker.setAbortCheck(abortCheck);
142+
}
143+
}
144+
137145
@Override
138146
public OnHeapHnswGraph getCompletedGraph() throws IOException {
139147
if (frozen == false) {

lucene/core/src/java/org/apache/lucene/util/hnsw/HnswGraphBuilder.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.lucene.search.TopDocs;
3434
import org.apache.lucene.search.knn.KnnSearchStrategy;
3535
import org.apache.lucene.util.FixedBitSet;
36+
import org.apache.lucene.util.IORunnable;
3637
import org.apache.lucene.util.InfoStream;
3738
import org.apache.lucene.util.hnsw.HnswUtil.Component;
3839

@@ -83,6 +84,7 @@ public class HnswGraphBuilder implements HnswBuilder {
8384
protected final HnswLock hnswLock;
8485

8586
protected InfoStream infoStream = InfoStream.getDefault();
87+
private IORunnable abortCheck;
8688
protected boolean frozen;
8789

8890
/**
@@ -209,6 +211,11 @@ public void setInfoStream(InfoStream infoStream) {
209211
this.infoStream = infoStream;
210212
}
211213

214+
@Override
215+
public void setAbortCheck(IORunnable abortCheck) {
216+
this.abortCheck = abortCheck;
217+
}
218+
212219
@Override
213220
public OnHeapHnswGraph getCompletedGraph() throws IOException {
214221
if (!frozen) {
@@ -280,6 +287,9 @@ private void addGraphNodeInternal(int node, UpdateableRandomVectorScorer scorer,
280287
if (frozen) {
281288
throw new IllegalStateException("Graph builder is already frozen");
282289
}
290+
if (abortCheck != null) {
291+
abortCheck.run();
292+
}
283293
final int nodeLevel = getRandomGraphLevel(ml, random);
284294
// first add nodes to all levels
285295
for (int level = nodeLevel; level >= 0; level--) {

lucene/core/src/java/org/apache/lucene/util/hnsw/IncrementalHnswGraphMerger.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.lucene.util.BitSet;
3434
import org.apache.lucene.util.Bits;
3535
import org.apache.lucene.util.FixedBitSet;
36+
import org.apache.lucene.util.IORunnable;
3637
import org.apache.lucene.util.InfoStream;
3738

3839
/**
@@ -46,6 +47,7 @@ public class IncrementalHnswGraphMerger implements HnswGraphMerger {
4647
protected final RandomVectorScorerSupplier scorerSupplier;
4748
protected final int M;
4849
protected final int beamWidth;
50+
protected final IORunnable abortCheck;
4951

5052
protected List<GraphReader> graphReaders = new ArrayList<>();
5153
protected GraphReader largestGraphReader;
@@ -71,10 +73,26 @@ protected record GraphReader(
7173
*/
7274
public IncrementalHnswGraphMerger(
7375
FieldInfo fieldInfo, RandomVectorScorerSupplier scorerSupplier, int M, int beamWidth) {
76+
this(fieldInfo, scorerSupplier, M, beamWidth, null);
77+
}
78+
79+
/**
80+
* @param fieldInfo FieldInfo for the field being merged
81+
* @param abortCheck optional check invoked before every node insertion during graph construction;
82+
* may throw {@link org.apache.lucene.index.MergePolicy.MergeAbortedException} to abort the
83+
* build when the surrounding merge has been aborted, or null
84+
*/
85+
public IncrementalHnswGraphMerger(
86+
FieldInfo fieldInfo,
87+
RandomVectorScorerSupplier scorerSupplier,
88+
int M,
89+
int beamWidth,
90+
IORunnable abortCheck) {
7491
this.fieldInfo = fieldInfo;
7592
this.scorerSupplier = scorerSupplier;
7693
this.M = M;
7794
this.beamWidth = beamWidth;
95+
this.abortCheck = abortCheck;
7896
}
7997

8098
/**
@@ -211,6 +229,9 @@ public OnHeapHnswGraph merge(
211229
KnnVectorValues mergedVectorValues, InfoStream infoStream, int maxOrd) throws IOException {
212230
HnswBuilder builder = createBuilder(mergedVectorValues, maxOrd);
213231
builder.setInfoStream(infoStream);
232+
if (abortCheck != null) {
233+
builder.setAbortCheck(abortCheck);
234+
}
214235
return builder.build(maxOrd);
215236
}
216237

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.lucene.index;
18+
19+
import java.util.Random;
20+
import java.util.concurrent.CountDownLatch;
21+
import java.util.concurrent.TimeUnit;
22+
import java.util.concurrent.atomic.AtomicReference;
23+
import org.apache.lucene.codecs.lucene99.Lucene99HnswVectorsFormat;
24+
import org.apache.lucene.document.Document;
25+
import org.apache.lucene.document.Field;
26+
import org.apache.lucene.document.KnnFloatVectorField;
27+
import org.apache.lucene.document.StringField;
28+
import org.apache.lucene.store.Directory;
29+
import org.apache.lucene.tests.util.LuceneTestCase;
30+
import org.apache.lucene.tests.util.TestUtil;
31+
import org.apache.lucene.util.InfoStream;
32+
33+
/**
34+
* Tests that aborting a merge (e.g. via {@link IndexWriter#rollback()}) promptly interrupts HNSW
35+
* graph construction instead of blocking until the entire graph is built.
36+
*/
37+
public class TestHnswMergeAbort extends LuceneTestCase {
38+
39+
private static final int DIM = 96;
40+
private static final int SEGMENTS = 4;
41+
private static final int DOCS_PER_SEGMENT = 12_000;
42+
private static final int BEAM_WIDTH = 250;
43+
44+
/**
45+
* Every segment carries more than {@code IncrementalHnswGraphMerger#DELETE_PCT_THRESHOLD}
46+
* deletions, so no source graph is eligible as a base and the merged graph is rebuilt from
47+
* scratch via {@code HnswGraphBuilder#addVectors}.
48+
*/
49+
public void testRollbackDuringFullRebuildMerge() throws Exception {
50+
doTestRollbackDuringMerge(true);
51+
}
52+
53+
/**
54+
* All segments are deletion-free, so the merged graph is produced by joining the source graphs
55+
* via {@code MergingHnswGraphBuilder}.
56+
*/
57+
public void testRollbackDuringGraphJoinMerge() throws Exception {
58+
doTestRollbackDuringMerge(false);
59+
}
60+
61+
private void doTestRollbackDuringMerge(boolean withDeletes) throws Exception {
62+
try (Directory dir = newDirectory()) {
63+
IndexWriterConfig cfg = new IndexWriterConfig();
64+
cfg.setCodec(TestUtil.alwaysKnnVectorsFormat(new Lucene99HnswVectorsFormat(16, BEAM_WIDTH)));
65+
cfg.setMergePolicy(NoMergePolicy.INSTANCE);
66+
try (IndexWriter w = new IndexWriter(dir, cfg)) {
67+
Random r = random();
68+
int id = 0;
69+
for (int s = 0; s < SEGMENTS; s++) {
70+
for (int i = 0; i < DOCS_PER_SEGMENT; i++, id++) {
71+
Document doc = new Document();
72+
doc.add(new StringField("id", Integer.toString(id), Field.Store.NO));
73+
float[] v = new float[DIM];
74+
for (int j = 0; j < DIM; j++) {
75+
v[j] = r.nextFloat();
76+
}
77+
doc.add(new KnnFloatVectorField("v", v));
78+
w.addDocument(doc);
79+
}
80+
w.flush();
81+
}
82+
if (withDeletes) {
83+
// delete half of every segment, above the 40% base-graph eligibility threshold
84+
for (int d = 0; d < id; d += 2) {
85+
w.deleteDocuments(new Term("id", Integer.toString(d)));
86+
}
87+
}
88+
w.commit();
89+
}
90+
91+
CountDownLatch buildStarted = new CountDownLatch(1);
92+
InfoStream latching =
93+
new InfoStream() {
94+
@Override
95+
public void message(String component, String message) {
96+
if ("HNSW".equals(component) && message.startsWith("build graph")) {
97+
buildStarted.countDown();
98+
}
99+
}
100+
101+
@Override
102+
public boolean isEnabled(String component) {
103+
return "HNSW".equals(component);
104+
}
105+
106+
@Override
107+
public void close() {}
108+
};
109+
110+
IndexWriterConfig cfg2 = new IndexWriterConfig();
111+
cfg2.setCodec(TestUtil.alwaysKnnVectorsFormat(new Lucene99HnswVectorsFormat(16, BEAM_WIDTH)));
112+
cfg2.setMergeScheduler(new ConcurrentMergeScheduler());
113+
cfg2.setInfoStream(latching);
114+
IndexWriter w2 = new IndexWriter(dir, cfg2);
115+
AtomicReference<Throwable> mergeFailure = new AtomicReference<>();
116+
Thread merger =
117+
new Thread(
118+
() -> {
119+
try {
120+
w2.forceMerge(1);
121+
} catch (Throwable t) {
122+
mergeFailure.set(t);
123+
}
124+
});
125+
merger.start();
126+
try {
127+
assertTrue(
128+
"HNSW graph construction never started", buildStarted.await(120, TimeUnit.SECONDS));
129+
long t0 = System.nanoTime();
130+
w2.rollback();
131+
long rollbackMillis = (System.nanoTime() - t0) / 1_000_000;
132+
assertTrue(
133+
"rollback() blocked for "
134+
+ rollbackMillis
135+
+ " ms waiting for HNSW graph construction to finish",
136+
rollbackMillis < 5_000);
137+
} finally {
138+
merger.join(TimeUnit.MINUTES.toMillis(5));
139+
assertFalse("merge thread did not terminate", merger.isAlive());
140+
}
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)