ArulJerald commented on PR #3991: URL: https://github.com/apache/iceberg-python/pull/3991#issuecomment-5729717696
Rewrote this per the discussion — the cache key is now (table_uuid, manifest_path) instead of a content comparison, and the equality-check has been removed entirely. @rambleraptor this directly addresses the cost concern: a genuine cache hit is back to a plain O(1) tuple lookup, no comparison against the cached record. I benchmarked it against an append on a table with ~2000 snapshots — identical number of metadata rebuilds before and after (22 vs 22), so this adds no measurable overhead on the write path. @Fokko to clarify the framing — I'm not claiming a manifest mutates in place; that would violate the spec and nothing here contradicts it. The actual scenario is a process-wide cache with no attribution to a table: two unrelated ManifestFile reads (different tables, or even two unrelated test fixtures reusing a placeholder path) landing on the same cache key. Nothing was mutated; the cache just had no way to tell "same manifest, correctly reused" apart from "different manifest, coincidentally same path," because the key was a bare path with no owner attached. Also: a caller that doesn't supply a table_uuid (e.g. any external code still on the old two-arg Snapshot.manifests(io) signature) now bypasses the cache entirely rather than falling into a shared unattributed bucket — so there's no remaining pathway for two tables to share an entry, attributed or not. One caveat worth flagging: table-uuid is optional in format v1, so a legacy v1 table without one gets a fresh UUID generated on every metadata parse (via default_factory) rather than None — it's never unattributed, just unstable across reloads. That means caching doesn't carry over across a refresh()/reload for that specific table, but it can never collide with another table's entries either way. Correctness-safe, just a smaller caching win for that edge case. -- 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]
