Skip to content

Commit c87fcd4

Browse files
author
Igor Motov
committed
Fix silent brute force fallback at aligned vector dimensions
cuvsDatasetMakePadded rejects a device matrix already at CAGRA's required stride, so every field whose dimension is a multiple of four failed its CAGRA build and was silently indexed as brute force. Pick the factory that matches the layout, and log the fallback so the next one is visible. See #2482
1 parent 2140532 commit c87fcd4

8 files changed

Lines changed: 204 additions & 4 deletions

File tree

java/cuvs-java/src/main/java/com/nvidia/cuvs/CagraIndex.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,23 @@ static CagraIndex merge(CagraIndex[] indexes, CagraIndexParams mergeParams) thro
336336
return CuVSProvider.provider().mergeCagraIndexes(indexes, mergeParams);
337337
}
338338

339+
/**
340+
* Reports whether the rows of {@code dataset} already sit at the row stride CAGRA requires, which
341+
* is the row length in bytes rounded up to a 16 byte boundary.
342+
*
343+
* <p>Use it to pick between the two padded dataset factories: a matrix that is already padded has
344+
* to go through {@link #makePaddedDatasetView(CuVSMatrix)}, because cuVS rejects a request to
345+
* copy it into padded storage it already occupies, and one that is not has to go through
346+
* {@link #makePaddedDataset(CuVSMatrix)}.
347+
*
348+
* @param dataset the matrix to inspect
349+
* @return true when the rows are already padded the way CAGRA requires
350+
*/
351+
static boolean isPaddedDataset(CuVSMatrix dataset) {
352+
Objects.requireNonNull(dataset);
353+
return CuVSProvider.provider().isCagraPaddedDataset(dataset);
354+
}
355+
339356
/**
340357
* Builder helps configure and create an instance of {@link CagraIndex}.
341358
*/

java/cuvs-java/src/main/java/com/nvidia/cuvs/spi/CuVSProvider.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,25 @@ default CagraIndex mergeCagraIndexes(CagraIndex[] indexes, CagraIndexParams merg
188188
return mergeCagraIndexes(indexes);
189189
}
190190

191+
/**
192+
* Reports whether the rows of {@code dataset} already sit at the row stride CAGRA requires, which
193+
* is the row length in bytes rounded up to a 16 byte boundary.
194+
*
195+
* <p>This is the question that decides which of the two padded dataset factories a caller has to
196+
* use: {@link CagraIndex#makePaddedDatasetView(CuVSMatrix)} for a device matrix that is already at
197+
* that stride, and {@link CagraIndex#makePaddedDataset(CuVSMatrix)} for one that is not. Asking
198+
* for the wrong one is an error rather than an inefficiency, and the stride of a matrix is not
199+
* visible outside this library, so callers cannot answer it for themselves.
200+
*
201+
* @param dataset the matrix to inspect
202+
* @return true when the rows are already padded the way CAGRA requires
203+
* @throws UnsupportedOperationException if this provider cannot answer
204+
*/
205+
default boolean isCagraPaddedDataset(CuVSMatrix dataset) {
206+
throw new UnsupportedOperationException(
207+
"Padded layout detection is not supported by " + getClass().getName());
208+
}
209+
191210
/**
192211
* Creates a device-backed multi-partition filter handle from the pre-packed combined bitset.
193212
* Per-partition bit offsets are recomputed inside cuVS from the index sizes.

java/cuvs-java/src/main/java/com/nvidia/cuvs/spi/UnsupportedProvider.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public CagraIndex mergeCagraIndexes(CagraIndex[] indexes) {
8585
throw new UnsupportedOperationException(reasons);
8686
}
8787

88+
@Override
89+
public boolean isCagraPaddedDataset(CuVSMatrix dataset) {
90+
throw new UnsupportedOperationException(reasons);
91+
}
92+
8893
@Override
8994
public CuVSMatrix.Builder<CuVSHostMatrix> newHostMatrixBuilder(
9095
long size, long dimensions, CuVSMatrix.DataType dataType) {

java/cuvs-java/src/main/java22/com/nvidia/cuvs/internal/CagraIndexImpl.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,19 @@ private static int elementSizeBytes(CuVSMatrix.DataType dataType) {
263263
};
264264
}
265265

266+
/**
267+
* True when the matrix row width matches CAGRA's required padded width for its logical column
268+
* count and element type. The stride lives on the internal matrix type, so this is the only place
269+
* that can answer the question; {@link com.nvidia.cuvs.CagraIndex#isPaddedDataset} routes here
270+
* through the provider.
271+
*/
272+
public static boolean isPaddedDataset(CuVSMatrix dataset) {
273+
if (!(dataset instanceof CuVSMatrixInternal datasetInternal)) {
274+
throw new IllegalArgumentException("dataset must be a CuVSMatrixInternal matrix");
275+
}
276+
return isCagraPaddedLayout(datasetInternal);
277+
}
278+
266279
/**
267280
* True when the matrix row width matches CAGRA's required padded width for its
268281
* logical column count and element type.

java/cuvs-java/src/main/java22/com/nvidia/cuvs/spi/JDKProvider.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,11 @@ public CagraIndex mergeCagraIndexes(CagraIndex[] indexes, CagraIndexParams merge
317317
return CagraIndexImpl.merge(indexes, mergeParams);
318318
}
319319

320+
@Override
321+
public boolean isCagraPaddedDataset(CuVSMatrix dataset) {
322+
return CagraIndexImpl.isPaddedDataset(dataset);
323+
}
324+
320325
@Override
321326
public GPUInfoProvider gpuInfoProvider() {
322327
return new GPUInfoProviderImpl();

java/cuvs-lucene/src/main/java/com/nvidia/cuvs/lucene/CuVS2510GPUVectorsWriter.java

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,14 @@ private void writeFieldInternal(FieldInfo fieldInfo, List<float[]> vectors) thro
206206
writeCagraIndex(cagraIndexOutputStream, cagraDataset);
207207
} catch (Throwable t) {
208208
// Fallback to brute force in a few cases, for now.
209+
// Log it to make it more obvious that this is what is happening.
210+
info(
211+
infoStream,
212+
COMPONENT,
213+
"CAGRA build failed for field \""
214+
+ fieldInfo.name
215+
+ "\", falling back to a brute force index: "
216+
+ t);
209217
Utils.handleThrowableWithIgnore(t, t.getMessage());
210218
indexType = IndexType.BRUTE_FORCE;
211219
}
@@ -248,10 +256,23 @@ private void writeCagraIndex(OutputStream os, CuVSMatrix dataset) throws Throwab
248256
.withDataset(dataset)
249257
.withIndexParams(params)
250258
.build();
251-
var deviceVectors = dataset.toDevice(getCuVSResourcesInstance());
252-
var indexDataset = index.makePaddedDataset(deviceVectors)) {
253-
index.updateDataset(indexDataset);
254-
index.serialize(os);
259+
var deviceVectors = dataset.toDevice(getCuVSResourcesInstance())) {
260+
/*
261+
* cuVS rejects makePaddedDataset for a device matrix whose rows already sit at the required
262+
* stride, and asks for a view over that storage instead. Copying would be pointless there
263+
* anyway, so pick the factory that matches the layout.
264+
*/
265+
if (CagraIndex.isPaddedDataset(deviceVectors)) {
266+
try (var indexDatasetView = index.makePaddedDatasetView(deviceVectors)) {
267+
index.updateDataset(indexDatasetView);
268+
index.serialize(os);
269+
}
270+
} else {
271+
try (var indexDataset = index.makePaddedDataset(deviceVectors)) {
272+
index.updateDataset(indexDataset);
273+
index.serialize(os);
274+
}
275+
}
255276
}
256277
}
257278

java/cuvs-lucene/src/main/java/com/nvidia/cuvs/lucene/FilterCuVSProvider.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public CagraIndex mergeCagraIndexes(CagraIndex[] arg0) throws Throwable {
8585
return delegate.mergeCagraIndexes(arg0);
8686
}
8787

88+
@Override
89+
public boolean isCagraPaddedDataset(CuVSMatrix arg0) {
90+
return delegate.isCagraPaddedDataset(arg0);
91+
}
92+
8893
@Override
8994
public GPUInfoProvider gpuInfoProvider() {
9095
return delegate.gpuInfoProvider();
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package com.nvidia.cuvs.lucene;
6+
7+
import static com.nvidia.cuvs.lucene.ThreadLocalCuVSResourcesProvider.isSupported;
8+
9+
import com.nvidia.cuvs.lucene.CuVS2510GPUVectorsWriter.IndexType;
10+
import java.io.IOException;
11+
import java.util.ArrayList;
12+
import java.util.Collections;
13+
import java.util.List;
14+
import org.apache.lucene.document.Document;
15+
import org.apache.lucene.document.KnnFloatVectorField;
16+
import org.apache.lucene.index.IndexWriter;
17+
import org.apache.lucene.index.IndexWriterConfig;
18+
import org.apache.lucene.index.VectorSimilarityFunction;
19+
import org.apache.lucene.store.Directory;
20+
import org.apache.lucene.tests.util.LuceneTestCase;
21+
import org.apache.lucene.tests.util.LuceneTestCase.SuppressSysoutChecks;
22+
import org.apache.lucene.tests.util.TestUtil;
23+
import org.apache.lucene.util.InfoStream;
24+
import org.junit.Assume;
25+
import org.junit.Test;
26+
27+
/**
28+
* A CAGRA row is padded to a 16 byte boundary, so a device matrix whose dimension is already a
29+
* multiple of four sits at the required stride. cuVS refuses to build an owning padded copy of such
30+
* a matrix and asks for a view instead, and the writer swallows a failed CAGRA build by falling
31+
* back to a brute force index. The two together are silent: search keeps returning correct results
32+
* while nothing on the GPU is a CAGRA index any more.
33+
*
34+
* <p>These tests pin the dimensions on both sides of that boundary.
35+
*/
36+
@SuppressSysoutChecks(bugUrl = "")
37+
public class TestCagraIndexAtAlignedDimensions extends LuceneTestCase {
38+
39+
@Test
40+
public void testCagraIsBuiltAtAnAlignedDimension() throws IOException {
41+
// 128 floats is 512 bytes, an exact multiple of the 16 byte CAGRA row alignment.
42+
assertCagraIsBuilt(128);
43+
}
44+
45+
@Test
46+
public void testCagraIsBuiltAtAnUnalignedDimension() throws IOException {
47+
// 127 floats is not, so the writer has to fall back to an owning padded copy.
48+
assertCagraIsBuilt(127);
49+
}
50+
51+
/** Indexes a segment of the given dimension and fails if the CAGRA build did not survive it. */
52+
private void assertCagraIsBuilt(int dimension) throws IOException {
53+
Assume.assumeTrue("Requires a GPU", isSupported());
54+
55+
RecordingInfoStream infoStream = new RecordingInfoStream();
56+
try (Directory directory = newDirectory()) {
57+
IndexWriterConfig config =
58+
new IndexWriterConfig()
59+
.setCodec(
60+
TestUtil.alwaysKnnVectorsFormat(
61+
new CuVS2510GPUVectorsFormat(
62+
new GPUSearchParams.Builder().withIndexType(IndexType.CAGRA).build())))
63+
.setInfoStream(infoStream);
64+
65+
try (IndexWriter writer = new IndexWriter(directory, config)) {
66+
for (int i = 0; i < 64; i++) {
67+
float[] vector = new float[dimension];
68+
for (int d = 0; d < dimension; d++) {
69+
vector[d] = random().nextFloat();
70+
}
71+
Document doc = new Document();
72+
doc.add(new KnnFloatVectorField("vector", vector, VectorSimilarityFunction.EUCLIDEAN));
73+
writer.addDocument(doc);
74+
}
75+
writer.commit();
76+
}
77+
}
78+
79+
assertTrue(
80+
"The CAGRA build fell back to brute force at dimension "
81+
+ dimension
82+
+ ", messages: "
83+
+ infoStream.messages(),
84+
infoStream.cagraBuildFailures().isEmpty());
85+
}
86+
87+
/** An InfoStream that keeps the messages, so that a test can tell which index type was built. */
88+
private static class RecordingInfoStream extends InfoStream {
89+
90+
private final List<String> messages = Collections.synchronizedList(new ArrayList<>());
91+
92+
@Override
93+
public void message(String component, String message) {
94+
messages.add(component + ": " + message);
95+
}
96+
97+
@Override
98+
public boolean isEnabled(String component) {
99+
return true;
100+
}
101+
102+
@Override
103+
public void close() {}
104+
105+
List<String> messages() {
106+
synchronized (messages) {
107+
return List.copyOf(messages);
108+
}
109+
}
110+
111+
List<String> cagraBuildFailures() {
112+
return messages().stream().filter(message -> message.contains("CAGRA build failed")).toList();
113+
}
114+
}
115+
}

0 commit comments

Comments
 (0)