This is an automated email from the ASF dual-hosted git repository.
gortiz 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 af39d1a6c1f Fix leaked read lock and unknown-n-gram crash in
RealtimeNgramFilteringIndex (#19612)
af39d1a6c1f is described below
commit af39d1a6c1f34888aaeb7488f0c840e476985d12
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Mon Sep 28 15:10:31 2026 +0200
Fix leaked read lock and unknown-n-gram crash in
RealtimeNgramFilteringIndex (#19612)
---
.../invertedindex/RealtimeNgramFilteringIndex.java | 27 ++++++++-----
.../RealtimeNgramFilteringIndexTest.java | 44 ++++++++++++++++++++++
2 files changed, 61 insertions(+), 10 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndex.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndex.java
index 51373512e00..a01430932be 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndex.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndex.java
@@ -109,23 +109,30 @@ public class RealtimeNgramFilteringIndex implements
MutableTextIndex {
/// @param searchQuery as a literal string
@Override
public MutableRoaringBitmap getDocIds(String searchQuery) {
- _readLock.lock();
+ // Generated outside the read lock: it only reads the query string and the
immutable n-gram length settings.
Iterable<String> ngrams = generateNgrams(searchQuery);
if (!ngrams.iterator().hasNext()) {
return null; // No n-grams generated, return null.
}
- ArrayList<MutableRoaringBitmap> bitmapLst = new ArrayList<>();
+ _readLock.lock();
try {
+ MutableRoaringBitmap resultBitmap = null;
for (String ngram : ngrams) {
int dictId = _ngramToDictIdMapping.getInt(ngram);
- bitmapLst.add(_invertedIndex.getDocIds(dictId));
- }
- if (bitmapLst.isEmpty()) {
- return null; // No n-grams found in the index.
- }
- MutableRoaringBitmap resultBitmap = bitmapLst.get(0);
- for (int i = 1; i < bitmapLst.size(); i++) {
- resultBitmap.and(bitmapLst.get(i));
+ if (dictId < 0) {
+ // The n-gram has never been indexed, so no document can contain all
the n-grams of the search query.
+ return new MutableRoaringBitmap();
+ }
+ // getDocIds returns a private copy, so it is safe to intersect into
the first one.
+ MutableRoaringBitmap docIds = _invertedIndex.getDocIds(dictId);
+ if (resultBitmap == null) {
+ resultBitmap = docIds;
+ } else {
+ resultBitmap.and(docIds);
+ }
+ if (resultBitmap.isEmpty()) {
+ return resultBitmap;
+ }
}
return resultBitmap;
} finally {
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndexTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndexTest.java
index 2cf25954733..f221f26dc69 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndexTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeNgramFilteringIndexTest.java
@@ -21,12 +21,16 @@ package
org.apache.pinot.segment.local.realtime.impl.invertedindex;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
@@ -76,6 +80,46 @@ public class RealtimeNgramFilteringIndexTest {
testSelectionResults(_ngram2To2Index, ngramQuery, null);
}
+ @Test
+ public void testQueryWithNgramThatWasNeverIndexed() {
+ // "zq" never appears in the indexed documents, so nothing can match. This
used to throw
+ // IndexOutOfBoundsException because the unknown n-gram resolved to
dictionary id -1.
+ MutableRoaringBitmap docIds = _ngram2To3Index.getDocIds("zq");
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+
+ // A query mixing a known and an unknown n-gram must also match nothing.
+ docIds = _ngram2To3Index.getDocIds("drezq");
+ assertNotNull(docIds);
+ assertTrue(docIds.isEmpty());
+ }
+
+ @Test(timeOut = 30_000L)
+ public void testReadLockReleasedWhenNoNgramIsGenerated()
+ throws Exception {
+ RealtimeNgramFilteringIndex index = new RealtimeNgramFilteringIndex("col",
2, 3);
+ try {
+ index.add("andrew");
+
+ // The search query is shorter than the minimum n-gram length, so no
n-gram is generated. This used to return
+ // while still holding the read lock, which blocked every later write
forever.
+ assertNull(index.getDocIds("r"));
+
+ CountDownLatch indexed = new CountDownLatch(1);
+ Thread writer = new Thread(() -> {
+ index.add("drew");
+ indexed.countDown();
+ });
+ writer.start();
+ assertTrue(indexed.await(10L, TimeUnit.SECONDS), "Writer thread is
blocked by a leaked read lock");
+ writer.join();
+
+ assertEquals(index.getDocIds("drew").getCardinality(), 2);
+ } finally {
+ index.close();
+ }
+ }
+
private List<String> getTextData() {
return Arrays.asList("andrew", "drerew", "draw", "dr", "dr and rew", "ew
re");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
