This is an automated email from the ASF dual-hosted git repository.
xiangfu0 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 a986489af49 Preserve queryableDocIds tracking when preloading upsert
segment with no valid docs (#18527)
a986489af49 is described below
commit a986489af495d8d41f9816a62f7defea5dc2bfc1
Author: Chaitanya Deepthi <[email protected]>
AuthorDate: Mon May 18 20:30:50 2026 -0700
Preserve queryableDocIds tracking when preloading upsert segment with no
valid docs (#18527)
* Make sure the queryable doc ids are not null when the valid doc ids are
empty
* Add unit tests for the regression
* Simplify the code
---
.../upsert/BasePartitionUpsertMetadataManager.java | 4 +-
...rrentMapPartitionUpsertMetadataManagerTest.java | 60 ++++++++++++++++++++++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
index 6df010ae0e8..f0c8f850b89 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java
@@ -437,7 +437,9 @@ public abstract class BasePartitionUpsertMetadataManager
implements PartitionUps
if (validDocIds.isEmpty()) {
_logger.info("Skip preloading segment: {} without valid doc, current
primary key count: {}",
segment.getSegmentName(), getNumPrimaryKeys());
- segment.enableUpsert(this, new ThreadSafeMutableRoaringBitmap(), null);
+ ThreadSafeMutableRoaringBitmap queryableDocIds =
+ (_deleteRecordColumn == null) ? null : new
ThreadSafeMutableRoaringBitmap();
+ segment.enableUpsert(this, new ThreadSafeMutableRoaringBitmap(),
queryableDocIds);
return;
}
if (isTTLEnabled() && !_comparisonColumns.isEmpty()) {
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerTest.java
index bbba657b985..926af5e34ce 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerTest.java
@@ -71,6 +71,7 @@ import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.pinot.util.TestUtils;
+import org.mockito.ArgumentCaptor;
import org.mockito.MockedConstruction;
import org.roaringbitmap.buffer.MutableRoaringBitmap;
import org.testng.annotations.AfterClass;
@@ -85,6 +86,7 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.*;
@@ -1356,6 +1358,64 @@ public class
ConcurrentMapPartitionUpsertMetadataManagerTest {
}
}
+ /**
+ * Regression test: when preloading an immutable segment whose validDocIds
snapshot is empty and the table is
+ * configured with a deleteRecordColumn, doPreloadSegment's fast path must
initialize queryableDocIds to an
+ * empty bitmap (not null). Passing null would set _queryableDocIds = null
on the segment, causing subsequent
+ * doTakeSnapshot rounds to skip persisting the queryableDocIds bitmap file
and leaving the on-disk snapshot
+ * frozen at its pre-restart contents.
+ */
+ @Test
+ public void testPreloadSegmentEmptyValidDocIdsWithDeleteColumn() {
+
_contextBuilder.setEnableSnapshot(true).setDeleteRecordColumn(DELETE_RECORD_COLUMN);
+ ConcurrentMapPartitionUpsertMetadataManager upsertMetadataManager =
+ new ConcurrentMapPartitionUpsertMetadataManager(REALTIME_TABLE_NAME,
0, _contextBuilder.build());
+
+ ImmutableSegmentImpl segment = mock(ImmutableSegmentImpl.class);
+ when(segment.getSegmentName()).thenReturn("emptyValidDocIdsSegment");
+
when(segment.loadDocIdsFromSnapshot(V1Constants.VALID_DOC_IDS_SNAPSHOT_FILE_NAME)).thenReturn(
+ new MutableRoaringBitmap());
+
+ upsertMetadataManager.preloadSegment(segment);
+
+ ArgumentCaptor<ThreadSafeMutableRoaringBitmap> validDocIdsCaptor =
+ ArgumentCaptor.forClass(ThreadSafeMutableRoaringBitmap.class);
+ ArgumentCaptor<ThreadSafeMutableRoaringBitmap> queryableDocIdsCaptor =
+ ArgumentCaptor.forClass(ThreadSafeMutableRoaringBitmap.class);
+ verify(segment).enableUpsert(eq(upsertMetadataManager),
validDocIdsCaptor.capture(),
+ queryableDocIdsCaptor.capture());
+ assertNotNull(validDocIdsCaptor.getValue());
+
assertTrue(validDocIdsCaptor.getValue().getMutableRoaringBitmap().isEmpty());
+ assertNotNull(queryableDocIdsCaptor.getValue(),
+ "queryableDocIds must be non-null when deleteRecordColumn is
configured");
+
assertTrue(queryableDocIdsCaptor.getValue().getMutableRoaringBitmap().isEmpty());
+ }
+
+ /**
+ * Companion to {@link #testPreloadSegmentEmptyValidDocIdsWithDeleteColumn}:
when deleteRecordColumn is not
+ * configured, the fast path should still pass null for queryableDocIds
since queryable tracking is disabled.
+ */
+ @Test
+ public void testPreloadSegmentEmptyValidDocIdsWithoutDeleteColumn() {
+ _contextBuilder.setEnableSnapshot(true);
+ ConcurrentMapPartitionUpsertMetadataManager upsertMetadataManager =
+ new ConcurrentMapPartitionUpsertMetadataManager(REALTIME_TABLE_NAME,
0, _contextBuilder.build());
+
+ ImmutableSegmentImpl segment = mock(ImmutableSegmentImpl.class);
+ when(segment.getSegmentName()).thenReturn("emptyValidDocIdsSegment");
+
when(segment.loadDocIdsFromSnapshot(V1Constants.VALID_DOC_IDS_SNAPSHOT_FILE_NAME)).thenReturn(
+ new MutableRoaringBitmap());
+
+ upsertMetadataManager.preloadSegment(segment);
+
+ ArgumentCaptor<ThreadSafeMutableRoaringBitmap> queryableDocIdsCaptor =
+ ArgumentCaptor.forClass(ThreadSafeMutableRoaringBitmap.class);
+ verify(segment).enableUpsert(eq(upsertMetadataManager),
any(ThreadSafeMutableRoaringBitmap.class),
+ queryableDocIdsCaptor.capture());
+ assertNull(queryableDocIdsCaptor.getValue(),
+ "queryableDocIds must be null when deleteRecordColumn is not
configured");
+ }
+
@Test
public void testAddRecordWithDeleteColumn()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
