talatuyarer commented on PR #17280: URL: https://github.com/apache/iceberg/pull/17280#issuecomment-5644324421
Thanks for this PR @Guosmilesmile. I agree that lookup joins are a critical missing piece for Iceberg. I know few teams which were migrating Hive dimension tables to Iceberg lose this capability today. However, I have two major concerns: 1. **Dependency Conflict:** Bundling `rocksdbjni` risks conflicts with Flink's existing RocksDB state backend. I recommend removing the RocksDB dependency and landing the memory-backed cache first to ensure stability. 2. **Leverage FLIP-221:** Please use [FLIP-221](https://cwiki.apache.org/confluence/spaces/FLINK/pages/211880884/FLIP-221+Abstraction+for+lookup+source+cache+and+metric)'s `FullCachingLookupProvider` instead of a custom implementation. It handles eager loading to prevent lookup stalls, provides standardized metrics/configuration, and supports native reload strategies. Additionally, please consider adding snapshot pinning for visibility and implementing a formal failure policy for reloads. For large tables, I recommend exploring [FLIP-462](https://cwiki.apache.org/confluence/spaces/FLINK/pages/309496789/FLIP-462+Support+Custom+Data+Distribution+for+Input+Stream+of+Lookup+Join) lookup shuffles in a follow-up instead of embedding RocksDB. -- 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]
