backupccl: prevent OR from linking in virtual file pointing to empty key space#122918
backupccl: prevent OR from linking in virtual file pointing to empty key space#122918craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
56bc559 to
5c6c520
Compare
|
unrelated test flake |
5c6c520 to
bedcce3
Compare
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
| } | ||
| if !entry.Span.Contains(file.BackupFileEntrySpan) { | ||
| return errors.AssertionFailedf("file span %s is not contained in restore span %s", file.BackupFileEntrySpan, entry.Span) | ||
| } |
There was a problem hiding this comment.
one thought: we don't really clean up linked ssts if the link phase fails partway through. I feel fine with this for private preview. Better to fail loudly then silently corrupt the db.
We could gen spans before the actual link phase but i dont feel like optimizing UX private preview restore rn.
There was a problem hiding this comment.
Let's log file.Path here as well to make it easier to investigate if it happens.
There was a problem hiding this comment.
We believe entry.Span == restoringSubspan, right? Should we check that too just to be sure?
stevendanna
left a comment
There was a problem hiding this comment.
Just some minor nits.
A bit of a bummer to have to bring out such a big hammer, but it's better than the alternative.
| } | ||
| if !entry.Span.Contains(file.BackupFileEntrySpan) { | ||
| return errors.AssertionFailedf("file span %s is not contained in restore span %s", file.BackupFileEntrySpan, entry.Span) | ||
| } |
There was a problem hiding this comment.
Let's log file.Path here as well to make it easier to investigate if it happens.
| return nil, errors.Newf("restore target contains two distinct spans with table id %d. Online restore cannot handle this as it may make an empty file span", tableID) | ||
| } |
There was a problem hiding this comment.
If you fancied we could return an unimplemented error here with a reference to an issue.
To prevent reading from a virtual file that points to empty key space, this patch fails an online restore if any of the target key spans are not table spans; for example, if a table had a dropped index. Informs cockroachdb#122176 Release note: none
bedcce3 to
195c733
Compare
…p files One way online restore can link a virtual file that points to empty key space in the backing file is if the restoreSpanEntry does not fully contain one of its backup files. This patch fails the link phase of online restore if this condition is detected. Epic: none Release note: none
195c733 to
3abb0e5
Compare
|
TFTR! bors r=dt |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build failed (retrying...): |
See individual commits.