-
Notifications
You must be signed in to change notification settings - Fork 406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cache by schema version #6847
Cache by schema version #6847
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, modulo the concerns I have about the changes to _config_cache
invalidation
edb/server/dbview/dbview.pyx
Outdated
state.append( | ||
{"name": '__dbver__', "value": dbver, "type": 'C'}) | ||
{"name": '__ver__', "value": ver, "type": 'C'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are we actually reading this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/edgedb/edgedb/blob/669ba34661ad04a605b12f745fa4b92122557570/edb/server/protocol/execute.pyx#L88-L91 and https://github.com/edgedb/edgedb/blob/669ba34661ad04a605b12f745fa4b92122557570/edb/server/protocol/execute.pyx#L253-L256 are the relevant places.
It doesn't get used directly, but changes in it are used to force a state sync
This reverts commit 669ba34.
Also added comments to explain how to handle compilation with a concurrent schema change.
Drop integer
dbver
and replace it with the UUIDschema_version
, so that we can persist the query cache under consistent keys in the next steps.dbver
was bumped on config changes butschema_version
is only for user schema. We handle this difference by adding such config into the compile cache key / PG state key.get_session_config()
for cache key should be optimized to honoraffects_compilation
, this is fixed in a future persistent cache PR.