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.

Sign up for GitHub

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

Jump to bottom

backupccl: add SHOW BACKUP ... with incremental_location for locality aware backups #82912

Closed
msbutler opened this issue Jun 14, 2022 · 1 comment · Fixed by #95562
Closed

backupccl: add SHOW BACKUP ... with incremental_location for locality aware backups #82912

msbutler opened this issue Jun 14, 2022 · 1 comment · Fixed by #95562
Assignees
@msbutler
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery

Comments

@msbutler
Copy link
Collaborator

msbutler commented Jun 14, 2022 •
edited by cockroach-jira-scripts
Loading

Currently, SHOW BACKUP works with the incremental_location parameter or for locality aware backups, but not for locality aware backups with the incremental_location parameter. This requires renovating SHOW BACKUP's implementation in sql.y to look more like BACKUP or RESTORE's more flexible sql.y implementation.

Jira issue: CRDB-16724
@msbutler msbutler added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 14, 2022
@msbutler msbutler self-assigned this Jun 14, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jun 14, 2022

cc @cockroachdb/bulk-io

@msbutler msbutler removed their assignment Jan 13, 2023
@msbutler msbutler mentioned this issue Jan 13, 2023
Merged
@msbutler msbutler self-assigned this Jan 19, 2023
msbutler added a commit to msbutler/cockroach that referenced this issue Jan 23, 2023
@msbutler
backupccl: explicitly parse SHOW BACKUP options
4d665b9 
Fixes: cockroachdb#82912

Release note (sql change): Previously, SHOW BACKUP options would get parsed as
`kv_options`, which meant that a user could not pass multiple values to a show
backup option, causing feature gaps in SHOW BACKUP relative to BACKUP and
RESTORE. This patch rewrites the show backup option parser, closing the
following feature gaps: 1. A user can now pass and check multiple KMS URIs in
SHOW BACKUP 2. A user can pass locality aware incremental_locations, allowing a
user to also pass the check_files parameter to a locality aware backup chain
that also specifies the backup incremental location.

Note that while this patch introduces a couple new words to the CRDB SQL
syntax, the same SHOW BACKUP options should remain documented, specifically:
- [public option] -> value
- AS_JSON -> N/A
- CHECK_FILES -> N/A
- INCREMENTAL_LOCATION -> string, with potentially multiple uris
- DEBUG_IDS -> N/A
- KMS -> string, with potentially multiple uris
- PRIVILEGES -> N/A
- ENCRYPTION_PASSPHRASE -> string
@msbutler msbutler mentioned this issue Jan 23, 2023
Merged
msbutler added a commit to msbutler/cockroach that referenced this issue Jan 26, 2023
@msbutler
backupccl: explicitly parse SHOW BACKUP options
3f72655 
Fixes: cockroachdb#82912

Release note (sql change): Previously, SHOW BACKUP options would get parsed as
`kv_options`, which meant that a user could not pass multiple values to a show
backup option, causing feature gaps in SHOW BACKUP relative to BACKUP and
RESTORE. This patch rewrites the show backup option parser, closing the
following feature gaps: 1. A user can now pass and check multiple KMS URIs in
SHOW BACKUP 2. A user can pass locality aware incremental_locations, allowing a
user to also pass the check_files parameter to a locality aware backup chain
that also specifies the backup incremental location.

Note that while this patch introduces a couple new words to the CRDB SQL
syntax, the same SHOW BACKUP options should remain documented, specifically:
- [public option] -> value
- AS_JSON -> N/A
- CHECK_FILES -> N/A
- INCREMENTAL_LOCATION -> string, with potentially multiple uris
- DEBUG_IDS -> N/A
- KMS -> string, with potentially multiple uris
- PRIVILEGES -> N/A
- ENCRYPTION_PASSPHRASE -> string
craig bot pushed a commit that referenced this issue Jan 27, 2023
@msbutler @ecwall
Merge #95562 #95745
5f0c56e 
95562: backupccl: explicitly parse SHOW BACKUP options r=benbardin a=msbutler

Fixes: #82912

Release note (sql change): Previously, SHOW BACKUP options would get parsed as
`kv_options`, which meant that a user could not pass multiple values to a show
backup option, causing feature gaps in SHOW BACKUP relative to BACKUP and
RESTORE. This patch rewrites the show backup option parser, closing the
following feature gaps:
 1. A user can now pass and check multiple KMS URIs in
SHOW BACKUP 
2. A user can pass locality aware incremental_locations, allowing a
user to also pass the check_files parameter to a locality aware backup chain
that also specifies the backup incremental location.

Note that while this patch introduces a couple new words to the CRDB SQL
syntax, the same SHOW BACKUP options should remain documented, specifically:
- [public option] -> value
- AS_JSON -> N/A
- CHECK_FILES -> N/A
- INCREMENTAL_LOCATION -> string, with potentially multiple uris
- DEBUG_IDS -> N/A
- KMS -> string, with potentially multiple uris
- PRIVILEGES -> N/A
- ENCRYPTION_PASSPHRASE -> string

95745: sql: remove redundant session iteration r=xinhaoz,yuzefovich a=ecwall

Fixes #95743

Improves session/query cancelation with the following
1) Replaces session scanning by session ID with map lookup.
2) Replaces active query scanning by query ID with map lookup
   (session containing query to cancel is still scanned for).
3) Does not serialize entire session to get session username or id.

Informs #77676

77676 was closed but some test cases incorrectly mentioned that addressing
77676 fixed them. This PR correctly fixes these test cases.

Release note: None


Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
@craig craig bot closed this as completed in e6f47a0 Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@msbutler msbutler

Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-disaster-recovery
Projects
No open projects
Disaster Recovery Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

backupccl: explicitly parse SHOW BACKUP options msbutler/cockroach
1 participant
@msbutler