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

backupccl: support SHOW BACKUP [RANGES|FILES] #26450

Merged
merged 3 commits into from
Jun 6, 2018

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jun 6, 2018

This makes debugging a broken backup substantially easier.

Release note (enterprise change): Introduce SHOW BACKUP RANGES and SHOW
BACKUP FILES, which show details about the ranges and files,
respectively, which comprise a backup.

benesch added 2 commits June 5, 2018 22:16
The last reference to opt_collate_clause was removed in cockroachdb#24958. Normally
this causes goyacc to throw a "rule not reduced" error, but for some
reason it's not happening until I make some unrelated changes. (Probably
a bug in goyacc.)

Take the opportunity to clean up the naming scheme, too: opt_collate is
used where optional collation is supported, while opt_collate_unimpl is
used where optional collation is not supported.

Release note: None
@benesch benesch requested review from dt, maddyblue, bobvawter and a team June 6, 2018 06:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable


fn: func(desc BackupDescriptor, resultsCh chan<- tree.Datums) {
for _, span := range desc.Spans {
resultsCh <- tree.Datums{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are redoing this, might as well do it correctly for all of them: plumb a ctx down here and do the send in a select that is also doing a recv on ctx.Done. Or maybe we definitely don't need that? Seems like it wouldn't hurt and isn't hard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that these things aren't emitted incrementally, I'm going to adjust the shower interface to return a slice of tree.Datums and then do it properly in the hook fn. Fix incoming.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fixed. PTAL.

This makes debugging a broken backup substantially easier.

Release note (enterprise change): Introduce SHOW BACKUP RANGES and SHOW
BACKUP FILES, which show details about the ranges and files,
respectively, which comprise a backup.
@benesch
Copy link
Contributor Author

benesch commented Jun 6, 2018

bors r=mjibson,dt

craig bot pushed a commit that referenced this pull request Jun 6, 2018
26450: backupccl: support SHOW BACKUP [RANGES|FILES] r=mjibson,dt a=benesch

This makes debugging a broken backup substantially easier.

Release note (enterprise change): Introduce SHOW BACKUP RANGES and SHOW
BACKUP FILES, which show details about the ranges and files,
respectively, which comprise a backup.

Co-authored-by: Nikhil Benesch <[email protected]>
@craig
Copy link
Contributor

craig bot commented Jun 6, 2018

Build succeeded

@craig craig bot merged commit 74b0bf7 into cockroachdb:master Jun 6, 2018
@benesch benesch deleted the backup-details branch June 18, 2018 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants