Skip to content
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

sql: add new builtins to save and restore a session #68464

Closed
rafiss opened this issue Aug 5, 2021 · 4 comments · Fixed by #68792
Closed

sql: add new builtins to save and restore a session #68464

rafiss opened this issue Aug 5, 2021 · 4 comments · Fixed by #68792
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Aug 5, 2021

To allow SQL pods to scale down and back up, without user impact, we should add two new builtins:

  1. one that “saves” (or “pickles”) the session state
  2. and another that “restores” (or “unpickles”) the session state.

This state can just be returned directly to the caller (in this case sqlproxy) in some encoded form. sqlproxy is in charge of managing this encoded state.

The initial implementation of these builtins only needs to include the values of all session variables in the state.

Behavior:

  • Sessions will not be suspended when there is an open transaction. Therefore, the "save" builtin should return an error if it's called in the middle of a transaction.
  • Consider that these builtins will be running inside of the user's session. We could add something to make sure the user matches user in the saved session state, but I don't think that's necessary. (Essentially, this first version of the command is a shortcut for SHOWing each session var, and then SETing them all later.)

Out of scope for this issue:

  • Including prepared statements in the session state. (We possibly may not even need this ever, since I believe drivers auto re-prepare statements, but more investigation needed.)
  • Temporary tables

Epic CRDB-1464

@rafiss rafiss added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 5, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Aug 5, 2021
@otan
Copy link
Contributor

otan commented Aug 6, 2021

Rudimentary research:

@otan
Copy link
Contributor

otan commented Aug 6, 2021

ah, sequences are already serialisable.

@otan
Copy link
Contributor

otan commented Aug 6, 2021

What guarantees do we have about migrating settings between different version of CockroachDB?

@otan
Copy link
Contributor

otan commented Aug 6, 2021

ah wait, there's this thing for sequences:

// SequenceCache stores sequence values which have been cached using the
// CACHE sequence option.
SequenceCache SequenceCache

craig bot pushed a commit that referenced this issue Aug 12, 2021
68525: sql: convert LocalOnlySessionData to protobuf  r=rafiss a=otan

See individual commits for details.

Refs: #68464

Co-authored-by: Oliver Tan <[email protected]>
@craig craig bot closed this as completed in 53a8df0 Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants