xiangfu0 opened a new pull request, #20114: URL: https://github.com/apache/pinot/pull/20114
This lets any `SegmentDirectory` implementation serve the mapped segment metadata added in #19998, not just `SegmentLocalFSDirectory`. Today the serving load switches to mapped metadata through an `instanceof SegmentLocalFSDirectory` check, and the mapped `SegmentMetadataImpl` constructor is private behind the static `loadMapped()` factories, which always return the base class. A directory for segments whose index files live outside the local segment directory can't opt in, and neither can one whose metadata subclasses `SegmentMetadataImpl`. Three small changes: - `SegmentDirectory#loadMappedMetadata()` is a new no-op hook. `ImmutableSegmentLoader.load(SegmentDirectory, ...)` calls it on any directory when mapped metadata is enabled, at the same point it already switched local directories: after preprocessing, before the segment serves. So preprocessing keeps running on eager metadata for every implementation. `SegmentLocalFSDirectory`'s existing method now overrides it, and its behavior doesn't change. - The mapped `SegmentMetadataImpl` constructor is now `protected`. It behaves exactly like `loadMapped(File, ReadMode, EagerSnapshot)`, so the caller owns the mapping and closes it. The reader's layout rules don't change: canonical files are found under `v3/` first, and index sizes come only from `v3/index_map`. - `detachedCopy()` takes the index directory from `getIndexDir()` instead of the field, so a subclass that reports a null directory still reports null in its detached copy. Nothing changes with mapped metadata disabled, and the new method and constructor are additions, so existing implementations and callers are unaffected. Tests: - `testServingLoadSwitchesAnySegmentDirectoryToMappedMetadata` loads a mocked, non-local `SegmentDirectory` and checks that the hook runs once with mapped metadata enabled and not at all with it disabled. - `testMappedMetadataSubclass` opens a subclass that overrides `getIndexDir()` over a converted v3 segment. It checks that the subclass is mapped and reports the same index sizes as eager metadata, that its detached copy is a plain, unmapped instance whose index directory is still null, and that closing it releases the mapping. All 20 `SegmentMetadataImplTest` and 5 `SegmentLocalFSDirectoryTest` cases pass, and spotless, license and checkstyle are clean on `pinot-segment-spi` and `pinot-segment-local`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
