From d4519159e7db10cce0a2ccb68c88cc43e571a5ea Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Wed, 22 Nov 2023 09:02:02 -0800 Subject: [PATCH] Move rate limiter reset to defer block (#4722) Move `ResetLimiter()` call to defer block to ensure that it gets called even if the backup fails. Without this change, any following mailbox backups may get artificially throttled by limiter. --- #### 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 - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/operations/backup.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 49bd5d7f0..5095ee8db 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -210,6 +210,12 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) { // Select an appropriate rate limiter for the service. ctx = op.bp.SetRateLimiter(ctx, op.Selectors.PathService(), op.Options) + // For exchange, rate limits are enforced on a mailbox level. Reset the + // rate limiter so that it doesn't accidentally throttle following mailboxes. + // This is a no-op if we are using token bucket limiter since it refreshes + // tokens on a fixed per second basis. + defer graph.ResetLimiter(ctx) + // Check if the protected resource has the service enabled in order for us // to run a backup. enabled, err := op.bp.IsServiceEnabled( @@ -328,12 +334,6 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) { logger.Ctx(ctx).Infow("completed backup", "results", op.Results) } - // For exchange, rate limits are enforced on a mailbox level. Reset the - // rate limiter so that it doesn't accidentally throttle following mailboxes. - // This is a no-op if we are using token bucket limiter since it refreshes - // tokens on a fixed per second basis. - graph.ResetLimiter(ctx) - return op.Errors.Failure() }