From 02108fab950ee4626424dc3792e40bae900c6bd4 Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Tue, 14 Nov 2023 11:45:34 -0800 Subject: [PATCH] Remove unused control option for disabling concurrency limiter (#4677) Concurrency limiter is stable and has been in use since several months. We had added a killswitch to disable it for safety reasons. It's not being used by SDK consumers, and hidden in corso cli. This PR removes the `--disable-concurrency-limiter` flag & associated control option. --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- src/cli/backup/exchange.go | 1 - src/cli/backup/exchange_test.go | 2 - src/cli/flags/options.go | 69 ++++++++------------ src/cli/flags/testdata/flags.go | 11 ++-- src/cli/utils/options.go | 1 - src/cli/utils/options_test.go | 3 - src/internal/m365/service/exchange/backup.go | 4 +- src/pkg/control/options.go | 4 -- 8 files changed, 33 insertions(+), 62 deletions(-) diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 683ca4357..83640a0bc 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -82,7 +82,6 @@ func addExchangeCommands(cmd *cobra.Command) *cobra.Command { flags.AddFetchParallelismFlag(c) flags.AddDisableDeltaFlag(c) flags.AddEnableImmutableIDFlag(c) - flags.AddDisableConcurrencyLimiterFlag(c) flags.AddDeltaPageSizeFlag(c) flags.AddGenericBackupFlags(c) diff --git a/src/cli/backup/exchange_test.go b/src/cli/backup/exchange_test.go index 194e6a8d2..f21757dd0 100644 --- a/src/cli/backup/exchange_test.go +++ b/src/cli/backup/exchange_test.go @@ -108,7 +108,6 @@ func (suite *ExchangeUnitSuite) TestBackupCreateFlags() { // bool flags "--" + flags.DisableDeltaFN, "--" + flags.EnableImmutableIDFN, - "--" + flags.DisableConcurrencyLimiterFN, }, flagsTD.PreparedGenericBackupFlags(), flagsTD.PreparedProviderFlags(), @@ -125,7 +124,6 @@ func (suite *ExchangeUnitSuite) TestBackupCreateFlags() { assert.True(t, co.ToggleFeatures.ForceItemDataDownload) assert.True(t, co.ToggleFeatures.DisableDelta) assert.True(t, co.ToggleFeatures.ExchangeImmutableIDs) - assert.True(t, co.ToggleFeatures.DisableConcurrencyLimiter) flagsTD.AssertGenericBackupFlags(t, cmd) flagsTD.AssertProviderFlags(t, cmd) flagsTD.AssertStorageFlags(t, cmd) diff --git a/src/cli/flags/options.go b/src/cli/flags/options.go index 841e13169..5b0eaf215 100644 --- a/src/cli/flags/options.go +++ b/src/cli/flags/options.go @@ -5,38 +5,36 @@ import ( ) const ( - AlertsFN = "alerts" - DeltaPageSizeFN = "delta-page-size" - DisableConcurrencyLimiterFN = "disable-concurrency-limiter" - DisableDeltaFN = "disable-delta" - DisableIncrementalsFN = "disable-incrementals" - ForceItemDataDownloadFN = "force-item-data-download" - EnableImmutableIDFN = "enable-immutable-id" - FailFastFN = "fail-fast" - FailedItemsFN = "failed-items" - FetchParallelismFN = "fetch-parallelism" - NoStatsFN = "no-stats" - RecoveredErrorsFN = "recovered-errors" - NoPermissionsFN = "no-permissions" - RunModeFN = "run-mode" - SkippedItemsFN = "skipped-items" - SkipReduceFN = "skip-reduce" + AlertsFN = "alerts" + DeltaPageSizeFN = "delta-page-size" + DisableDeltaFN = "disable-delta" + DisableIncrementalsFN = "disable-incrementals" + ForceItemDataDownloadFN = "force-item-data-download" + EnableImmutableIDFN = "enable-immutable-id" + FailFastFN = "fail-fast" + FailedItemsFN = "failed-items" + FetchParallelismFN = "fetch-parallelism" + NoStatsFN = "no-stats" + RecoveredErrorsFN = "recovered-errors" + NoPermissionsFN = "no-permissions" + RunModeFN = "run-mode" + SkippedItemsFN = "skipped-items" + SkipReduceFN = "skip-reduce" ) var ( - DeltaPageSizeFV int - DisableConcurrencyLimiterFV bool - DisableDeltaFV bool - DisableIncrementalsFV bool - ForceItemDataDownloadFV bool - EnableImmutableIDFV bool - FailFastFV bool - FetchParallelismFV int - ListAlertsFV string - ListFailedItemsFV string - ListSkippedItemsFV string - ListRecoveredErrorsFV string - NoStatsFV bool + DeltaPageSizeFV int + DisableDeltaFV bool + DisableIncrementalsFV bool + ForceItemDataDownloadFV bool + EnableImmutableIDFV bool + FailFastFV bool + FetchParallelismFV int + ListAlertsFV string + ListFailedItemsFV string + ListSkippedItemsFV string + ListRecoveredErrorsFV string + NoStatsFV bool // RunMode describes the type of run, such as: // flagtest, dry, run. Should default to 'run'. RunModeFV string @@ -151,19 +149,6 @@ func AddEnableImmutableIDFlag(cmd *cobra.Command) { cobra.CheckErr(fs.MarkHidden(EnableImmutableIDFN)) } -// 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)) -} - // AddRunModeFlag adds the hidden --run-mode flag. func AddRunModeFlag(cmd *cobra.Command, persistent bool) { fs := cmd.Flags() diff --git a/src/cli/flags/testdata/flags.go b/src/cli/flags/testdata/flags.go index d3bea0c93..ce2fadefe 100644 --- a/src/cli/flags/testdata/flags.go +++ b/src/cli/flags/testdata/flags.go @@ -79,12 +79,11 @@ var ( FetchParallelism = "3" - FailFast = true - DisableIncrementals = true - ForceItemDataDownload = true - DisableDelta = true - EnableImmutableID = true - DisableConcurrencyLimiter = true + FailFast = true + DisableIncrementals = true + ForceItemDataDownload = true + DisableDelta = true + EnableImmutableID = true ) func WithFlags2( diff --git a/src/cli/utils/options.go b/src/cli/utils/options.go index fdb92aced..670d45ca5 100644 --- a/src/cli/utils/options.go +++ b/src/cli/utils/options.go @@ -26,7 +26,6 @@ func Control() control.Options { opt.ToggleFeatures.ForceItemDataDownload = flags.ForceItemDataDownloadFV opt.ToggleFeatures.DisableDelta = flags.DisableDeltaFV opt.ToggleFeatures.ExchangeImmutableIDs = flags.EnableImmutableIDFV - opt.ToggleFeatures.DisableConcurrencyLimiter = flags.DisableConcurrencyLimiterFV opt.Parallelism.ItemFetch = flags.FetchParallelismFV return opt diff --git a/src/cli/utils/options_test.go b/src/cli/utils/options_test.go index 919f33b3a..91d5d8b98 100644 --- a/src/cli/utils/options_test.go +++ b/src/cli/utils/options_test.go @@ -35,7 +35,6 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { assert.True(t, flags.NoPermissionsFV, flags.NoPermissionsFN) assert.True(t, flags.SkipReduceFV, flags.SkipReduceFN) assert.Equal(t, 2, flags.FetchParallelismFV, flags.FetchParallelismFN) - assert.True(t, flags.DisableConcurrencyLimiterFV, flags.DisableConcurrencyLimiterFN) assert.Equal(t, 499, flags.DeltaPageSizeFV, flags.DeltaPageSizeFN) }, } @@ -50,7 +49,6 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { flags.AddNoPermissionsFlag(cmd) flags.AddSkipReduceFlag(cmd) flags.AddFetchParallelismFlag(cmd) - flags.AddDisableConcurrencyLimiterFlag(cmd) flags.AddDeltaPageSizeFlag(cmd) // Test arg parsing for few args @@ -64,7 +62,6 @@ func (suite *OptionsUnitSuite) TestAddExchangeCommands() { "--" + flags.NoPermissionsFN, "--" + flags.SkipReduceFN, "--" + flags.FetchParallelismFN, "2", - "--" + flags.DisableConcurrencyLimiterFN, "--" + flags.DeltaPageSizeFN, "499", }) diff --git a/src/internal/m365/service/exchange/backup.go b/src/internal/m365/service/exchange/backup.go index e885ace18..d8893795b 100644 --- a/src/internal/m365/service/exchange/backup.go +++ b/src/internal/m365/service/exchange/backup.go @@ -50,11 +50,9 @@ func ProduceBackupCollections( bpc.Options.ToggleFeatures.DisableDelta = true } - // Turn on concurrency limiter middleware for exchange backups - // unless explicitly disabled through DisableConcurrencyLimiterFN cli flag graph.InitializeConcurrencyLimiter( ctx, - bpc.Options.ToggleFeatures.DisableConcurrencyLimiter, + true, bpc.Options.Parallelism.ItemFetch) cdps, canUsePreviousBackup, err := exchange.ParseMetadataCollections(ctx, bpc.MetadataCollections) diff --git a/src/pkg/control/options.go b/src/pkg/control/options.go index 7db2d9038..16f5436e7 100644 --- a/src/pkg/control/options.go +++ b/src/pkg/control/options.go @@ -83,10 +83,6 @@ type Toggles struct { 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"` - // PreviewBackup denotes that this backup contains a subset of information for // the protected resource. PreviewBackups are used to demonstrate value by // being quick to create.