backupccl: add TestOnlineRestoreS3 and TestOnlineRestoreBasic tests #114279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, if sendAddRemoteSSTs would return an error, the online restore would hang because the coordinator did not close the tracingAggregator channel. This patch fixes this. Epic: none Release note: none
|
This change is |
This comment was marked as outdated.
This comment was marked as outdated.
msbutler
force-pushed
the
butler-online-restore-unit
branch
from
d4c36f8 to
8661371
Compare
…pply This hack allows online restore to succeed. In the long run, I guess we'll need to know if the external sst has point or range keys in the provided span. Epic: none Release note: none
msbutler
force-pushed
the
butler-online-restore-unit
branch
2 times, most recently
from
aae055b to
ddf6c07
Compare
This patch adds TestOnlineRestoreS3 to our cloud unit test suite. To run locally, run: ``` ./dev test pkg/ccl/backupccl -f TestOnlineRestoreS3 -- --test_env=AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID --test_env=AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY --test_env=AWS_S3_BUCKET=cockroachdb-backup-cloud-nightly --test_env=COCKROACH_UNSAFE_RESTORE=true ``` This patch also adds TestOnlineRestoreBasic which runs online restore through nodelocal. To run locally, run: ``` ./dev test pkg/ccl/backupccl -f TestOnlineRestoreBasic -- --test_env=COCKROACH_UNSAFE_RESTORE=true ``` Both tests will be skipped in the nightlies until we pass COCKROACH_UNSAFE_RESTORE to their setup. Epic: none Release note: none
This patch removes the COCKROACH_UNSAFE_RESTORE env var which gates online restore. The gate no longer seems necesssary as all development will occur on master for the next few months. In addition, the env var makes it slightly annoying to test online restore. Epic: None Release note: None
msbutler
force-pushed
the
butler-online-restore-unit
branch
from
06ac99b to
49e8cd4
Compare
msbutler
commented
Nov 13, 2023
| @@ -655,6 +655,10 @@ func addSSTablePreApply( | |||
| Size: sst.BackingFileSize, | |||
| SmallestUserKey: start.Encode(), | |||
| LargestUserKey: end.Encode(), | |||
|
|
|||
| // TODO(msbutler): I guess we need to figure out if the backing external | |||
There was a problem hiding this comment.
@itsbilal Given the preso this morning, I guess I should reword this comment to: "we set this true even if the virtual SST's span covers no point keys in the remote sst, to avoid a pebble level panic during ingestion. At download time, if the backing file has no actual keys in the span, the virtual sst will be compacted away and no keys will download."
dt
approved these changes
Nov 13, 2023
|
TFTR! bors r=dt |
itsbilal
approved these changes
Nov 13, 2023
|
|
||
| // TODO(msbutler): I guess we need to figure out if the backing external | ||
| // file has point or range keys in the target span. | ||
| HasPointKey: true, |
There was a problem hiding this comment.
It's not possible to have range keys in backup SSTs at the moment right? So this might be sufficient and good enough for us.
| @@ -802,6 +802,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) { | |||
| } | |||
| addCfgOpt(storage.MaxSize(sizeInBytes)) | |||
| addCfgOpt(storage.CacheSize(cfg.CacheSize)) | |||
| addCfgOpt(storage.RemoteStorageFactory(cfg.ExternalStorageAccessor)) | |||
There was a problem hiding this comment.
I imagine this is test-only code seeing as it's under InMem, while for regular engines we already pass this cfg down below.
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds TestOnlineRestoreS3 to our cloud unit test suite. To run locally, run:
This patch also adds TestOnlineRestoreBasic which runs online restore through
nodelocal. To run locally, run:
Both tests will be skipped in the nightlies until we pass
COCKROACH_UNSAFE_RESTORE to their setup.
Epic: none
Release note: none