Akanksha-kedia commented on PR #18898: URL: https://github.com/apache/pinot/pull/18898#issuecomment-5236287622
Hi @J-HowHuang — thank you for the detailed suggestion! Your proposed approach has been implemented in the current revision. The PR now uses a V2 header layout exactly as you described, with the effective `fpp` stored at `FPP_OFFSET` immediately after the version field: ``` +------------------+---------------+--------------+-----------------------------+ | TYPE_VALUE (int) | VERSION (int) | FPP (double) | Guava bloom filter bytes... | +------------------+---------------+--------------+-----------------------------+ ``` All new bloom filter segments are written with `VERSION_V2 = 2`. The reader checks the version to decide which path to take: - **V2 segments**: read `fpp` directly from the header at `FPP_OFFSET` and compare with the configured value — no Guava internals involved. - **V1 legacy segments**: fpp change detection is skipped (graceful degradation); the segment upgrades to V2 naturally on the next rebuild triggered by other criteria. Regarding the broader consistency point raised in apache/pinot#18920: we've also added `readStoredIndexConfig()` / `setStoredIndexConfig()` utilities to `BaseIndexHandler` as a uniform `metadata.properties`-based alternative for index types that prefer not to embed config in their binary format. For the bloom filter, the header approach you suggested is the cleaner choice since it keeps the config self-contained in the index file itself — so this PR stays with the V2 header. -- 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]
