-
Notifications
You must be signed in to change notification settings - Fork 43
Merge native backup #4381
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
Merge native backup #4381
Conversation
2939c5c
to
df12f50
Compare
fae10ae
to
df12f50
Compare
@karol-kokoszka @VAveryanov8 Please take a look. This is important in terms of leveraging the new native backup api from Scylla 2025.2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
df12f50
to
c2862cf
Compare
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
Added a support for the api-hint parameter of scylla-manager backup. It is set by an SCT parameter of object_storage_upload_mode with an enum of ObjectStorageUploadMode: auto/rclone/native. refs: scylladb/scylla-manager#4381
89ee907
to
4b7e582
Compare
0743352
to
4dd7b64
Compare
New commits:
@VAveryanov8 @karol-kokoszka please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@karol-kokoszka please take a look! |
|
@Michal-Leszczynski Overall, looks very good. |
Yes, but I will also work on #4393 (in a separate PR) to make it more visible. |
This commit also uses sizesuffix.SizeSuffix instead of managerclient.SizeSuffix in command/repair. This type has been moved across modules, and we need to adjust for it.
1bdb0a0
to
3f5927c
Compare
Added 3 commits that needs to be squashed before merge: @karol-kokoszka please take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Please squash and merge. It's ok to to have more than 1 commit, but don't leave micro commits, rather rely on features.
Commits like:
[feat(restore): adjust model to recreated restore_run_progress]
[refactor(scyllaclient): remove unused ToCanonicalIP]
[feat(sstable): describe sstable specification]
Are not informative without the context. Linking the issue is not enough.
As there are no plans of adding http[s] scheme constants to http pkg, it should be fine to hardcode them instead of adding constants to some util pkg. Ref: golang/go#40587
Previously when using sctool, --skip-schema was set to --purge-only by mistake.
A separate column for Scylla task ID is needed because: - it has a different type from agent job ID - it makes it clear which API was used In case of restore run progress, we need it to be a part of the clustering key. Otherwise, we won't be able to store multiple run progresses with the same remote sstable dir and host in the DB. That's why we need to recreate the restore_run_progress table via MigrateToRestoreRunProgressWithSortKeyWithScyllaTaskID, which ensures that restore_run_progress data is not lost in the process. Since we are recreating the table, we can also rename manifest_path and versioned_progress to more suitable names.
Config is passed by value, so even if it's filled before registering provider, this change is not reflected in other places. Because of that, it's best to fill it at the beginning.
3f5927c
to
740dee3
Compare
This PR contains subset of changes from
ml/scylla-api-2
needed in order to support native backup api.The main change comes with replacing rclone
sync/movedir
api with native scyllastorage_service/backup
api when uploading snapshot-ed files into s3. This PR also makes some changes regarding configuring SM test env to support the native scylla backup api. The most interesting part from user perspective is the newsctool backup --method
flag (previously--api-hint
), which is documented as below:Initially it was supposed to be set to
rclone
by default, but I decided to go withauto
because:auto
before releaseFixes #4143
Fixes #4138
Fixes #4141
Fixes #4384
Fixes #4386
Fixes #4388