Skip to content

Core: Rebuild REST cached tables with current context#15954

Open
manuzhang wants to merge 1 commit intoapache:mainfrom
manuzhang:core/rest-cache-current-context
Open

Core: Rebuild REST cached tables with current context#15954
manuzhang wants to merge 1 commit intoapache:mainfrom
manuzhang:core/rest-cache-current-context

Conversation

@manuzhang
Copy link
Copy Markdown
Member

@manuzhang manuzhang commented Apr 12, 2026

This PR fixes a context-leak risk in REST table caching: after a 304 response, callers now get tables rebuilt with their own current auth/context rather than reusing potentially stale context captured by the original cached supplier.


Co-authored-by: @codex

@github-actions github-actions bot added the core label Apr 12, 2026
@singhpk234 singhpk234 self-requested a review April 12, 2026 18:35
@manuzhang manuzhang force-pushed the core/rest-cache-current-context branch from 4be5f29 to 1d8f408 Compare April 14, 2026 13:33
Co-authored-by: Codex <codex@openai.com>
@manuzhang manuzhang force-pushed the core/rest-cache-current-context branch from 1d8f408 to d7707bd Compare April 14, 2026 13:35
@gaborkaszab
Copy link
Copy Markdown
Contributor

Hey @manuzhang ,
Thanks for the PR. I started taking a look as the owner of the original code, but frankly I found the code somewhat complicated to understand. So before taking a deeper look, may I ask the motivation and the problem being solved in more details. What is the exact issue, what engine, what use-case, what error, what breaks, etc. Is this a real issue you see in some test/prod environment or a theoretical one that you think might break one time?
The reason I ask is that most of the engines use RESTSessionCatalog through a single RESTCatalog and as a result through the same session (and even many different users share the same session). I've checked Spark, Hive, Impala does this if I haven't missed anything, while Trino creates SessionContexts on its own.
Also, the idea to balance between tables being cached and credentials expiring, the general rule of thumb is to set the expiration on the cache way lower than the expiration of the credentials to guarantee that whenever getting a table from the cache, its creds will live for enough time to finish the query. Isn't this something that helps with you use-case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants