xiangfu0 commented on code in PR #19998:
URL: https://github.com/apache/pinot/pull/19998#discussion_r4429974792
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java:
##########
@@ -2196,7 +2196,8 @@ protected SegmentDirectory initSegmentDirectory(String
segmentName, long segment
SegmentDirectoryLoaderRegistry.getSegmentDirectoryLoader(indexLoadingConfig.getSegmentDirectoryLoader());
File indexDir =
getSegmentDataDir(segmentName, indexLoadingConfig.getSegmentTier(),
indexLoadingConfig.getTableConfig());
- return segmentDirectoryLoader.load(indexDir.toURI(), loaderContext);
+ return segmentDirectoryLoader.load(indexDir.toURI(),
indexLoadingConfig.isMappedSegmentMetadataEnabled()
+ ? loaderContext.withMappedMetadataEnabled() : loaderContext);
Review Comment:
Extending the flag here is safe precisely because the previous commit put
the reuse guard in build() rather than in caller discipline: fingerprints and
the IndexService identity must match open()'s fresh reads, so the worst case at
any call site is a wasted snapshot capture, never a stale reuse. With
initSegmentDirectory covered, the single-parse property now holds for the
common existing-segment startup path too.
##########
pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java:
##########
@@ -374,6 +381,54 @@ public void testReloadSegmentUseLocalCopy()
}
}
+ @Test
+ public void testMappedMetadataOnExistingSegmentLoadAndDirectoryReuseReload()
Review Comment:
Good end-to-end shape for this test: driving the real reload flow on the
local-copy path (createBackup never called), mutating metadata.properties
beforehand, and asserting the reloaded mapped view shows the fresh value is
exactly the leak a stale snapshot would produce — plus the loader-registry mock
pins that the flag actually reaches the directory loader.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
