This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 14235e1bc27 Skip iterative File.exists() check while answering which
column has text index and vector index (#19212)
14235e1bc27 is described below
commit 14235e1bc272c399270ddf80566c6898acba249c
Author: Jhow <[email protected]>
AuthorDate: Wed Aug 12 02:31:54 2026 +0800
Skip iterative File.exists() check while answering which column has text
index and vector index (#19212)
---
.../loader/invertedindex/TextIndexHandler.java | 18 +++-----
.../segment/store/SingleFileIndexDirectory.java | 12 +----
.../local/segment/store/TextIndexUtils.java | 44 ++++++++++++++++++
.../local/segment/store/VectorIndexUtils.java | 35 ++++++++++++++
.../store/SingleFileIndexDirectoryTest.java | 54 ++++++++++++++++++++++
5 files changed, 142 insertions(+), 21 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
index 0b49ab82937..008de7b9513 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
@@ -20,6 +20,7 @@ package
org.apache.pinot.segment.local.segment.index.loader.invertedindex;
import java.io.File;
import java.io.IOException;
+import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@@ -382,18 +383,13 @@ public class TextIndexHandler extends BaseIndexHandler {
File indexDir = _segmentDirectory.getSegmentMetadata().getIndexDir();
File segmentDirectory =
SegmentDirectoryPaths.segmentDirectoryFor(indexDir,
_segmentDirectory.getSegmentMetadata().getVersion());
- Set<String> columns = new HashSet<>();
- for (String column :
_segmentDirectory.getSegmentMetadata().getAllColumns()) {
- if (hasLegacyNativeTextIndex(indexDir, segmentDirectory, column)) {
- columns.add(column);
- }
+ Collection<String> allColumns =
_segmentDirectory.getSegmentMetadata().getAllColumns();
+ Set<String> columns = new
HashSet<>(TextIndexUtils.getColumnsWithLegacyNativeTextIndex(indexDir,
allColumns));
+ // Preserves the original short-circuit: the version subdirectory is only
consulted when it is a
+ // different directory from the segment root.
+ if (!segmentDirectory.equals(indexDir)) {
+
columns.addAll(TextIndexUtils.getColumnsWithLegacyNativeTextIndex(segmentDirectory,
allColumns));
}
return columns;
}
-
- private boolean hasLegacyNativeTextIndex(File indexDir, File
segmentDirectory, String column) {
- String legacyNativeTextIndexFile = column +
V1Constants.Indexes.DEPRECATED_NATIVE_TEXT_INDEX_FILE_EXTENSION;
- return new File(indexDir, legacyNativeTextIndexFile).exists()
- || (!segmentDirectory.equals(indexDir) && new File(segmentDirectory,
legacyNativeTextIndexFile).exists());
- }
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
index 3afcdb12b3e..2b4f1e6b3cc 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java
@@ -523,21 +523,13 @@ class SingleFileIndexDirectory extends
ColumnIndexDirectory {
Set<String> columns = new HashSet<>();
// TEXT_INDEX is not tracked via _columnEntries, so handled separately.
if (type == StandardIndexes.text()) {
- for (String column : _segmentMetadata.getAllColumns()) {
- if (TextIndexUtils.hasTextIndex(_segmentDirectory, column)) {
- columns.add(column);
- }
- }
+ columns.addAll(TextIndexUtils.getColumnsWithTextIndex(_segmentDirectory,
_segmentMetadata.getAllColumns()));
}
if (type == StandardIndexes.vector()) {
// Vector may live as a combined file (legacy /
storeInSegmentFile=false) or as a typed
// entry in columns.psf (storeInSegmentFile=true). Collect both. Removed
the early-return
// that previously hid consolidated entries from this view.
- for (String column : _segmentMetadata.getAllColumns()) {
- if (VectorIndexUtils.hasVectorIndex(_segmentDirectory, column)) {
- columns.add(column);
- }
- }
+
columns.addAll(VectorIndexUtils.getColumnsWithVectorIndex(_segmentDirectory,
_segmentMetadata.getAllColumns()));
}
for (IndexKey indexKey : _columnEntries.keySet()) {
if (indexKey._type == type) {
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
index c5b56825733..87ce1c3e836 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java
@@ -22,6 +22,7 @@ import java.io.File;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -113,6 +114,49 @@ public class TextIndexUtils {
//@formatter:on
}
+ /// Bulk form of [#hasTextIndex]: which of `columns` have a Lucene text
index in `segDir`.
+ ///
+ /// Prefer this over calling [#hasTextIndex] in a loop over a segment's
columns. It lists the
+ /// directory once rather than performing `columns × extensions` existence
probes. Finding
+ /// `<column><extension>` in the listing is equivalent to that path
existing, so the answer is the
+ /// same.
+ public static Set<String> getColumnsWithTextIndex(File segDir,
Collection<String> columns) {
+ Set<String> entries = listEntryNames(segDir);
+ Set<String> columnsWithIndex = new HashSet<>();
+ for (String column : columns) {
+ if (entries.contains(column + Indexes.LUCENE_TEXT_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.LUCENE_V9_TEXT_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.LUCENE_V99_TEXT_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.LUCENE_V912_TEXT_INDEX_FILE_EXTENSION)) {
+ columnsWithIndex.add(column);
+ }
+ }
+ return columnsWithIndex;
+ }
+
+ /// Which of `columns` still have a deprecated native text index file in
`segDir`.
+ ///
+ /// Listing-based for the same reason as [#getColumnsWithTextIndex]: the
caller asks this for every
+ /// column of a segment on every reload check, including the overwhelmingly
common case where no
+ /// column has ever had a native text index.
+ public static Set<String> getColumnsWithLegacyNativeTextIndex(File segDir,
Collection<String> columns) {
+ Set<String> entries = listEntryNames(segDir);
+ Set<String> columnsWithIndex = new HashSet<>();
+ for (String column : columns) {
+ if (entries.contains(column +
Indexes.DEPRECATED_NATIVE_TEXT_INDEX_FILE_EXTENSION)) {
+ columnsWithIndex.add(column);
+ }
+ }
+ return columnsWithIndex;
+ }
+
+ /// Names of the entries directly inside `dir`, files and directories alike
(a Lucene text index is
+ /// a directory), or empty when it cannot be listed.
+ private static Set<String> listEntryNames(File dir) {
+ String[] names = dir.list();
+ return names == null ? Set.of() : new HashSet<>(Arrays.asList(names));
+ }
+
public static List<String> extractStopWordsInclude(String colName,
Map<String, Map<String, String>> columnProperties) {
return extractStopWordsInclude(columnProperties.getOrDefault(colName,
null));
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/VectorIndexUtils.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/VectorIndexUtils.java
index e50d8dc15af..ecd54e4a995 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/VectorIndexUtils.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/VectorIndexUtils.java
@@ -22,6 +22,10 @@ import com.google.common.annotations.VisibleForTesting;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
import javax.annotation.Nullable;
import org.apache.commons.io.FileUtils;
import org.apache.lucene.codecs.lucene912.Lucene912Codec;
@@ -100,6 +104,37 @@ public class VectorIndexUtils {
|| new File(segDir, column +
Indexes.VECTOR_IVF_PQ_INDEX_FILE_EXTENSION).exists();
}
+ /// Bulk form of [#hasVectorIndex]: which of `columns` have a vector index
sidecar in `segDir`.
+ ///
+ /// Prefer this over calling [#hasVectorIndex] in a loop over a segment's
columns. It lists the
+ /// directory once rather than performing `columns × extensions` existence
probes. Finding
+ /// `<column><extension>` in the listing is equivalent to that path
existing, so the answer is the
+ /// same.
+ public static Set<String> getColumnsWithVectorIndex(File segDir,
Collection<String> columns) {
+ Set<String> entries = listEntryNames(segDir);
+ Set<String> columnsWithIndex = new HashSet<>();
+ for (String column : columns) {
+ if (entries.contains(column + Indexes.VECTOR_HNSW_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.VECTOR_V99_HNSW_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.VECTOR_V912_HNSW_INDEX_FILE_EXTENSION)
+ || entries.contains(column + Indexes.VECTOR_INDEX_FILE_EXTENSION)
+ || entries.contains(column + Indexes.VECTOR_V99_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.VECTOR_V912_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.VECTOR_IVF_FLAT_INDEX_FILE_EXTENSION)
+ || entries.contains(column +
Indexes.VECTOR_IVF_PQ_INDEX_FILE_EXTENSION)) {
+ columnsWithIndex.add(column);
+ }
+ }
+ return columnsWithIndex;
+ }
+
+ /// Names of the entries directly inside `dir`, files and directories alike
(an HNSW vector index is
+ /// a Lucene directory), or empty when it cannot be listed.
+ private static Set<String> listEntryNames(File dir) {
+ String[] names = dir.list();
+ return names == null ? Set.of() : new HashSet<>(Arrays.asList(names));
+ }
+
/// Returns `true` when the V1/V2 segment directory holds an IVF vector
index in the
/// combined-form extension (`.vector.ivfflat.combined.index` or
/// `.vector.ivfpq.combined.index`). The combined form is written by an IVF
creator run
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
index 92063a42c7c..3825363d5f9 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectoryTest.java
@@ -408,4 +408,58 @@ public class SingleFileIndexDirectoryTest implements
PinotBuffersAfterMethodChec
new HashSet<>(List.of("bar")));
}
}
+
+ /// Vector indexes may live either as a sibling file next to `columns.psf`
or as a typed entry
+ /// inside it, so `getColumnsWithIndex(vector())` has to union a filesystem
view with
+ /// `_columnEntries`. Discovery is purely name-based
(`<column><extension>`), which is what this
+ /// asserts — including the two ways a name-based scan can go wrong:
crediting a sidecar to a column
+ /// that is not in the segment, and mis-parsing a file whose name merely
*contains* an extension.
+ @Test
+ public void testGetColumnsWithVectorIndex()
+ throws Exception {
+ // Sidecar forms, one per extension family, to cover more than a single
spelling.
+ createEmptyFile("hnswCol" +
V1Constants.Indexes.VECTOR_HNSW_INDEX_FILE_EXTENSION);
+ createEmptyFile("ivfCol" +
V1Constants.Indexes.VECTOR_IVF_FLAT_INDEX_FILE_EXTENSION);
+ // Lucene-backed vector indexes are directories, not files — both must be
discovered.
+ FileUtils.forceMkdir(new File(TEMP_DIR, "dirCol" +
V1Constants.Indexes.VECTOR_V912_INDEX_FILE_EXTENSION));
+ // A sidecar for a column the segment does not declare must not be
reported.
+ createEmptyFile("strayCol" +
V1Constants.Indexes.VECTOR_HNSW_INDEX_FILE_EXTENSION);
+ // A name that merely contains an extension is not "<column><extension>"
for any real column, so
+ // it must not be credited to anything. This is the case a
suffix-stripping scan gets wrong.
+ createEmptyFile("notAColumn" +
V1Constants.Indexes.VECTOR_INDEX_FILE_EXTENSION + ".bak");
+
+ try (SingleFileIndexDirectory sfd = new SingleFileIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
+ // A consolidated vector index — tracked in _columnEntries rather than
on disk.
+ PinotDataBuffer buf = sfd.newBuffer("psfCol", StandardIndexes.vector(),
1024);
+ buf.putInt(0, 777);
+ }
+
+ when(_segmentMetadata.getAllColumns()).thenReturn(
+ new TreeSet<>(Arrays.asList("hnswCol", "ivfCol", "dirCol", "psfCol",
"plainCol")));
+ try (SingleFileIndexDirectory sfd = new SingleFileIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
+ assertEquals(sfd.getColumnsWithIndex(StandardIndexes.vector()),
+ new HashSet<>(Arrays.asList("hnswCol", "ivfCol", "dirCol",
"psfCol")));
+ }
+ }
+
+ /// A segment with no vector index anywhere reports none — the common case,
and the one that used to
+ /// cost `columns × extensions` existence probes to answer.
+ @Test
+ public void testGetColumnsWithVectorIndexWhenAbsent()
+ throws Exception {
+ try (SingleFileIndexDirectory sfd = new SingleFileIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
+ PinotDataBuffer buf = sfd.newBuffer("col1", StandardIndexes.forward(),
1024);
+ buf.putInt(0, 111);
+ }
+ when(_segmentMetadata.getAllColumns()).thenReturn(new
TreeSet<>(Arrays.asList("col1", "col2")));
+ try (SingleFileIndexDirectory sfd = new SingleFileIndexDirectory(TEMP_DIR,
_segmentMetadata, ReadMode.mmap)) {
+ assertEquals(sfd.getColumnsWithIndex(StandardIndexes.vector()),
Set.of());
+ assertEquals(sfd.getColumnsWithIndex(StandardIndexes.text()), Set.of());
+ }
+ }
+
+ private static void createEmptyFile(String name)
+ throws IOException {
+ FileUtils.touch(new File(TEMP_DIR, name));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
