From 4ce2f63da2cd1856f15795adbb3d655ef0bf842d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Leszczy=C5=84ski?= <2000michal@wp.pl> Date: Tue, 1 Oct 2024 14:43:46 +0200 Subject: [PATCH] feat(restore): support --transfers=0 for max speed restore and make it default Our experiments showed that the fastest file download was achieved for transfers=2*cpu_cnt. In order to make it easier for the user to use, a new, special value of --transfers=0 flag will take care of that. It is set as the default, because we aim to make not configured restore as fast as possible. --- docs/source/sctool/partials/sctool_restore.yaml | 3 ++- .../source/sctool/partials/sctool_restore_update.yaml | 3 ++- pkg/command/restore/cmd.go | 2 +- pkg/command/restore/res.yaml | 1 + pkg/service/restore/model.go | 11 +++++++---- pkg/service/restore/tables_worker.go | 8 ++++++-- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/source/sctool/partials/sctool_restore.yaml b/docs/source/sctool/partials/sctool_restore.yaml index f880d8a5c..3da78a00f 100644 --- a/docs/source/sctool/partials/sctool_restore.yaml +++ b/docs/source/sctool/partials/sctool_restore.yaml @@ -157,9 +157,10 @@ options: Timezone of --cron and --window flag values. The default value is taken from this system, namely 'TZ' envvar or '/etc/localtime' file. - name: transfers - default_value: "2" + default_value: "0" usage: | Sets the amount of file transfers to run in parallel when downloading files from backup location to Scylla node. + Set to 0 for the fastest download (results in setting transfers to 2*node_shard_count). - name: window default_value: '[]' usage: | diff --git a/docs/source/sctool/partials/sctool_restore_update.yaml b/docs/source/sctool/partials/sctool_restore_update.yaml index 142abaebe..f9e876078 100644 --- a/docs/source/sctool/partials/sctool_restore_update.yaml +++ b/docs/source/sctool/partials/sctool_restore_update.yaml @@ -155,9 +155,10 @@ options: Timezone of --cron and --window flag values. The default value is taken from this system, namely 'TZ' envvar or '/etc/localtime' file. - name: transfers - default_value: "2" + default_value: "0" usage: | Sets the amount of file transfers to run in parallel when downloading files from backup location to Scylla node. + Set to 0 for the fastest download (results in setting transfers to 2*node_shard_count). - name: window default_value: '[]' usage: | diff --git a/pkg/command/restore/cmd.go b/pkg/command/restore/cmd.go index 66a881310..de2b02e7c 100644 --- a/pkg/command/restore/cmd.go +++ b/pkg/command/restore/cmd.go @@ -80,7 +80,7 @@ func (cmd *command) init() { w.Unwrap().StringVarP(&cmd.snapshotTag, "snapshot-tag", "T", "", "") w.Unwrap().IntVar(&cmd.batchSize, "batch-size", 2, "") w.Unwrap().IntVar(&cmd.parallel, "parallel", 1, "") - w.Unwrap().IntVar(&cmd.transfers, "transfers", 2, "") + w.Unwrap().IntVar(&cmd.transfers, "transfers", 0, "") w.Unwrap().BoolVar(&cmd.allowCompaction, "allow-compaction", false, "") w.Unwrap().BoolVar(&cmd.restoreSchema, "restore-schema", false, "") w.Unwrap().BoolVar(&cmd.restoreTables, "restore-tables", false, "") diff --git a/pkg/command/restore/res.yaml b/pkg/command/restore/res.yaml index f7fd2f2cc..84400186d 100644 --- a/pkg/command/restore/res.yaml +++ b/pkg/command/restore/res.yaml @@ -35,6 +35,7 @@ parallel: | transfers: | Sets the amount of file transfers to run in parallel when downloading files from backup location to Scylla node. + Set to 0 for the fastest download (results in setting transfers to 2*node_shard_count). allow-compaction: | Defines if auto compactions should be running on Scylla nodes during restore. diff --git a/pkg/service/restore/model.go b/pkg/service/restore/model.go index 7977b4d1e..4d8cbc621 100644 --- a/pkg/service/restore/model.go +++ b/pkg/service/restore/model.go @@ -38,13 +38,16 @@ type Target struct { locationHosts map[Location][]string `json:"-"` } -const maxBatchSize = 0 +const ( + maxBatchSize = 0 + maxTransfers +) func defaultTarget() Target { return Target{ BatchSize: 2, Parallel: 0, - Transfers: 2, + Transfers: 0, Continue: true, } } @@ -64,8 +67,8 @@ func (t Target) validateProperties() error { if t.Parallel < 0 { return errors.New("parallel param has to be greater or equal to zero") } - if t.Transfers < 1 { - return errors.New("transfers param has to be greater than zero") + if t.Transfers < 0 { + return errors.New("transfers param has to be greater or equal to zero") } if t.RestoreSchema == t.RestoreTables { return errors.New("choose EXACTLY ONE restore type ('--restore-schema' or '--restore-tables' flag)") diff --git a/pkg/service/restore/tables_worker.go b/pkg/service/restore/tables_worker.go index f4c057e1f..07a43d91b 100644 --- a/pkg/service/restore/tables_worker.go +++ b/pkg/service/restore/tables_worker.go @@ -200,8 +200,12 @@ func (w *tablesWorker) stageRestoreData(ctx context.Context) error { } for _, h := range hosts { - if err := w.client.RcloneSetTransfers(ctx, h, w.target.Transfers); err != nil { - return errors.Wrapf(err, "set transfers on %s to %d", h, w.target.Transfers) + transfers := w.target.Transfers + if transfers == maxTransfers { + transfers = 2 * int(hostToShard[h]) + } + if err := w.client.RcloneSetTransfers(ctx, h, transfers); err != nil { + return errors.Wrapf(err, "set transfers on %s to %d", h, transfers) } }