From c38a43107962b509d07b79b72b9c4f3773315559 Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Mon, 24 Apr 2023 18:22:12 -0700 Subject: [PATCH] Add --disable-concurrency-limiter cli flag (#3203) The concurrency limiter middleware is enabled by default for exchange backups. Adding a hidden --disable-concurrency-limiter flag to disable this middleware. Reasons for adding this flag: We have done limited perf testing with the concurrency limiter. In addition, our understanding of exchange concurrency limits is also a bit fuzzy. This flag acts as a killswitch in case we start to see perf regressions in prod. I see this as a temporary flag. Ideally we would never have to use it. Also, once we have a better way to configure concurrency limiter, we can eliminate this flag. --- #### Does this PR need a docs update or release note? - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature #### Issue(s) * # #### Test Plan - [ ] :zap: Unit test --- src/cli/backup/exchange.go | 1 + src/cli/options/options.go | 31 ++++++++++++++----- src/cli/options/options_test.go | 5 +-- .../connector/exchange/data_collections.go | 7 +++-- src/pkg/control/options.go | 6 ++++ 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index b0eac0bdf..ee0070e25 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -89,6 +89,7 @@ func addExchangeCommands(cmd *cobra.Command) *cobra.Command { options.AddFailFastFlag(c) options.AddDisableIncrementalsFlag(c) options.AddEnableImmutableIDFlag(c) + options.AddDisableConcurrencyLimiterFlag(c) case listCommand: c, fs = utils.AddCommand(cmd, exchangeListCmd()) diff --git a/src/cli/options/options.go b/src/cli/options/options.go index 20f233b60..8c091b682 100644 --- a/src/cli/options/options.go +++ b/src/cli/options/options.go @@ -19,6 +19,7 @@ func Control() control.Options { opt.SkipReduce = skipReduceFV opt.ToggleFeatures.DisableIncrementals = disableIncrementalsFV opt.ToggleFeatures.ExchangeImmutableIDs = enableImmutableID + opt.ToggleFeatures.DisableConcurrencyLimiter = disableConcurrencyLimiterFV opt.Parallelism.ItemFetch = fetchParallelismFV return opt @@ -29,13 +30,14 @@ func Control() control.Options { // --------------------------------------------------------------------------- const ( - FailFastFN = "fail-fast" - FetchParallelismFN = "fetch-parallelism" - NoStatsFN = "no-stats" - RestorePermissionsFN = "restore-permissions" - SkipReduceFN = "skip-reduce" - DisableIncrementalsFN = "disable-incrementals" - EnableImmutableIDFN = "enable-immutable-id" + FailFastFN = "fail-fast" + FetchParallelismFN = "fetch-parallelism" + NoStatsFN = "no-stats" + RestorePermissionsFN = "restore-permissions" + SkipReduceFN = "skip-reduce" + DisableIncrementalsFN = "disable-incrementals" + EnableImmutableIDFN = "enable-immutable-id" + DisableConcurrencyLimiterFN = "disable-concurrency-limiter" ) var ( @@ -117,3 +119,18 @@ func AddEnableImmutableIDFlag(cmd *cobra.Command) { "Enable exchange immutable ID.") cobra.CheckErr(fs.MarkHidden(EnableImmutableIDFN)) } + +var disableConcurrencyLimiterFV bool + +// AddDisableConcurrencyLimiterFlag adds a hidden cli flag which, when set, +// removes concurrency limits when communicating with graph API. This +// flag is only relevant for exchange backups for now +func AddDisableConcurrencyLimiterFlag(cmd *cobra.Command) { + fs := cmd.Flags() + fs.BoolVar( + &disableConcurrencyLimiterFV, + DisableConcurrencyLimiterFN, + false, + "Disable concurrency limiter middleware. Default: false") + cobra.CheckErr(fs.MarkHidden(DisableConcurrencyLimiterFN)) +} diff --git a/src/cli/options/options_test.go b/src/cli/options/options_test.go index ae229396e..78617f3e1 100644 --- a/src/cli/options/options_test.go +++ b/src/cli/options/options_test.go @@ -32,6 +32,7 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { assert.True(t, restorePermissionsFV, RestorePermissionsFN) assert.True(t, skipReduceFV, SkipReduceFN) assert.Equal(t, 2, fetchParallelismFV, FetchParallelismFN) + assert.True(t, disableConcurrencyLimiterFV, DisableConcurrencyLimiterFN) }, } @@ -42,8 +43,8 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { AddDisableIncrementalsFlag(cmd) AddRestorePermissionsFlag(cmd) AddSkipReduceFlag(cmd) - AddFetchParallelismFlag(cmd) + AddDisableConcurrencyLimiterFlag(cmd) // Test arg parsing for few args cmd.SetArgs([]string{ @@ -53,8 +54,8 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { "--" + NoStatsFN, "--" + RestorePermissionsFN, "--" + SkipReduceFN, - "--" + FetchParallelismFN, "2", + "--" + DisableConcurrencyLimiterFN, }) err := cmd.Execute() diff --git a/src/internal/connector/exchange/data_collections.go b/src/internal/connector/exchange/data_collections.go index 8af42aee4..82204d9d6 100644 --- a/src/internal/connector/exchange/data_collections.go +++ b/src/internal/connector/exchange/data_collections.go @@ -182,8 +182,11 @@ func DataCollections( categories = map[path.CategoryType]struct{}{} ) - // TODO: Add hidden cli flag to disable this feature - graph.InitializeConcurrencyLimiter(ctrlOpts.Parallelism.ItemFetch) + // Turn on concurrency limiter middleware for exchange backups + // unless explicitly disabled through DisableConcurrencyLimiterFN cli flag + if !ctrlOpts.ToggleFeatures.DisableConcurrencyLimiter { + graph.InitializeConcurrencyLimiter(ctrlOpts.Parallelism.ItemFetch) + } cdps, err := parseMetadataCollections(ctx, metadata, errs) if err != nil { diff --git a/src/pkg/control/options.go b/src/pkg/control/options.go index dc547cbb8..de6e76efc 100644 --- a/src/pkg/control/options.go +++ b/src/pkg/control/options.go @@ -103,4 +103,10 @@ type Toggles struct { // immutable Exchange IDs. This is only safe to set if the previous backup for // incremental backups used immutable IDs or if a full backup is being done. ExchangeImmutableIDs bool `json:"exchangeImmutableIDs,omitempty"` + + RunMigrations bool `json:"runMigrations"` + + // DisableConcurrencyLimiter removes concurrency limits when communicating with + // graph API. This flag is only relevant for exchange backups for now + DisableConcurrencyLimiter bool `json:"disableConcurrencyLimiter,omitempty"` }