REST: Send If-None-Match on table refresh

[raunaqmorarka]

RESTTableOperations.refresh() always issued an unconditional GET and parsed the full metadata body, even though the same catalog already does freshness-aware loading for loadTable through RESTTableCache. Servers that honour If-None-Match (for example Polaris) answer with 304 Not Modified, so a refresh of an unchanged table only needs a header round trip.

RESTTableOperations now takes the ETag of the load response through its constructor, updates it from every successful refresh and commit response, sends it as If-None-Match on refresh(), and returns current() on a 304 without parsing a body. When the server sends no ETag, refresh behaves as before. The RESTTableCache entry is left untouched, since the ETag there pairs with the metadata captured by the cached supplier.

RESTSessionCatalog.newTableOps gets a new overload with an eTag parameter; the existing one is deprecated and delegates with null.

Note: commit() now uses the RESTClient.post overload that returns response headers. BaseHTTPClient supports it; a custom RESTClient relying only on the interface default would need to implement it.

Tests cover a conditional refresh returning 304, a changed table still refreshing fully and adopting the new ETag, a commit ETag being reused, servers that send no ETag, and the 304 path over the Jetty-backed test server.

[gaborkaszab]

Isn't this solving the same as #17832?

[raunaqmorarka]

Isn't this solving the same as #17832?

Different problem. #17832 restores the lazy snapshots supplier and the snapshots query parameter on refresh in refs mode. This PR makes refresh conditional with If-None-Match so an unchanged table gets a 304 and no metadata body is parsed. Both change refresh() and updateCurrentMetadata, so one will need a rebase after the other merges, but they are independent. They also fit together: with #17832 the refresh request carries the same query parameters as the load, so the ETag from loadTable matches on the first refresh, and a 304 leaves current() and its snapshots supplier untouched.

[gaborkaszab]

Hi @raunaqmorarka ,

I'm trying to refresh my memories since I was involved with this. Couple of questions:

1. Isn't the ETag redundant now: There is one kept in the table cache and one in ops. Also, they can get out of sync, right?
2. In terms of the table cache, this is an in-place operation, right? Executing ops.refresh() also refreshes the TableMetadata in the table cache through ops, right?
3. I recall one complication might be that credentials and configs are also returned by the get request not just table metadata and ETag. Since these are incorporated into FileIO and RESTClient, they are not refreshed, right? Since the age of the table in the cache is not affected here, probably we are fine with this, just want to double check.

cc @nastra @danielcweeks Might be a useful addition. I always dragged this as a potential future addition to freshness-aware loading. Would you mind also taking a look?

[raunaqmorarka]

1. The two ETags validate different metadata. The cache ETag belongs to the metadata captured in the cached supplier, and the ops ETag belongs to ops.current(). They diverge after a refresh or commit, but each stays paired with its own metadata. Writing the refreshed ETag back into the cache would be wrong, since the cached supplier still returns the load-time metadata and a 304 on loadTable would then hand out stale metadata. The cost of divergence is one extra full load on the next loadTable, which then replaces the cache entry.

2. Correct, it is not in place. The cached supplier creates a new RESTTableOperations on every get(), so ops.refresh() only updates that instance. The cache entry keeps the load-time metadata and ETag.

3. refresh() never consumed config or credentials from the response before this change either. Only loadTable feeds them into the table FileIO. A 304 has no body, so nothing changes on that path, and the cache entry with its FileIO is untouched.