Repository navigation
Conversation
bbfe47e to
69d9a9a
Compare
| }; | ||
| Ok(TableCache::builder() | ||
| .max_capacity(parse(REST_CATALOG_PROP_TABLE_CACHE_MAX_ENTRIES, 100)?) | ||
| .time_to_live(Duration::from_millis(parse( |
There was a problem hiding this comment.
Compared with Java's RESTTableCache: the property names, defaults, the (session id, identifier) key and the 304 handling all match. One difference: Java rejects rest-table-cache.expire-after-write-ms of 0 (Preconditions.checkArgument(expireAfterWriteMS > 0, "Invalid expire after write: zero or negative")), while here 0 is accepted. Should this reject 0 too, or document what 0 does?
There was a problem hiding this comment.
good catch, thanks. 0 is now rejected like in Java, since it's ambiguous (disable vs never expire) and max-entries=0 already turns the cache off. while there, also capped it at 1000 years, which moka would otherwise panic on.
There was a problem hiding this comment.
Thanks, the upper bound is a nice catch too.
Which issue does this PR close?
RestCatalog: support freshness-aware table loading (If-None-Match / 304 Not Modified), as Iceberg Java does#3340.What changes are included in this PR?
load_tablenever sentIf-None-Matchand parsed a body-less304as a200. Following Java (apache/iceberg#14398):ETag, keyed by session and identifier; a later load sendsIf-None-Matchand returns the cached table on304.rest-table-cache.max-entries(default 100, 0 disables) andrest-table-cache.expire-after-write-ms(default 5 minutes).Only load responses fill the cache, as in Java: the reference server's
ETagalso covers query parameters, so a create response's never matches a later load.Are these changes tested?
apache/iceberg-rest-fixture:latest: loads return200then304; after a commit, drop or re-create, the staleETagyields200or404instead of the cached table.AI Disclosure