From b464f31b389c9a7c45a5afe2b80b46f279ccf3a6 Mon Sep 17 00:00:00 2001 From: Jesper Hansen Date: Wed, 10 Jul 2019 20:59:18 +0200 Subject: [PATCH] Add progressive backoff function (#11497) * [MM-15267] Utils: add backoff function to allow retries (#10958) * [MM-15267] Utils: add unit test and update retry logic (#10958) * [MM-15267] Utils: Add three retries to ProgressiveRetry (#10958) * [MM-15267] Utils: add comments for progressive retry (#10958) * [MM-15267] Utils: add license header to newly added file (#10958) * [MM-15267] Utils: fix typo (#10958) * [MM-15267] Utils: inline callback in function call (#10958) * [MM-15267] Utils: remove type definition for backoff callback function (#10958) * [MM-15267] Utils: use lookup table for timeout duration (#10958) * [MM-15267] Utils: table driven unit tests for Progressive Backoff (#10958) * [MM-15267] Utils: simplify retry function (#10958) * [MM-15267] Utils: add assert and require packages to test file (#10958) * [MM-15267] Utils: revert changes in go.mod and go.sum (#10958) --- plugin/helpers_bots.go | 18 ++++++++---- utils/backoff.go | 26 ++++++++++++++++++ utils/backoff_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 utils/backoff.go create mode 100644 utils/backoff_test.go diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index 664a954d5d..28ed4df3f0 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -4,9 +4,8 @@ package plugin import ( - "time" - "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" "github.com/pkg/errors" ) @@ -17,11 +16,20 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) } // If we fail for any reason, this could be a race between creation of bot and - // retreval from anouther EnsureBot. Just try the basic retrieve existing again. + // retrieval from another EnsureBot. Just try the basic retrieve existing again. defer func() { if retBotId == "" || retErr != nil { - time.Sleep(time.Second) - botIdBytes, err := p.API.KVGet(BOT_USER_KEY) + var err error + var botIdBytes []byte + + err = utils.ProgressiveRetry(func() error { + botIdBytes, err = p.API.KVGet(BOT_USER_KEY) + if err != nil { + return err + } + return nil + }) + if err == nil && botIdBytes != nil { retBotId = string(botIdBytes) retErr = nil diff --git a/utils/backoff.go b/utils/backoff.go new file mode 100644 index 0000000000..be6dcc9a79 --- /dev/null +++ b/utils/backoff.go @@ -0,0 +1,26 @@ +// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "time" +) + +var backoffTimeouts = []time.Duration{50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond, 400 * time.Millisecond} + +// ProgressiveRetry executes a BackoffOperation and waits an increasing time before retrying the operation. +func ProgressiveRetry(operation func() error) error { + var err error + + for attempts := 0; attempts < len(backoffTimeouts); attempts++ { + err = operation() + if err == nil { + return nil + } + + time.Sleep(backoffTimeouts[attempts]) + } + + return err +} diff --git a/utils/backoff_test.go b/utils/backoff_test.go new file mode 100644 index 0000000000..1ebff82e73 --- /dev/null +++ b/utils/backoff_test.go @@ -0,0 +1,62 @@ +package utils + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProgressiveRetry(t *testing.T) { + var retries int + + type args struct { + operation func() error + } + tests := []struct { + name string + args args + wantErr bool + expectedRetries int + }{ + { + name: "Should fail and return error", + args: args{ + operation: func() error { + retries++ + return errors.New("Operation Failed") + }, + }, + wantErr: true, + expectedRetries: 6, + }, + { + name: "Should succeed after two retries", + args: args{ + operation: func() error { + retries++ + if retries == 2 { + return nil + } + + return errors.New("Operation Failed") + }, + }, + wantErr: false, + expectedRetries: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + retries = 0 + + err := ProgressiveRetry(tt.args.operation) + if !tt.wantErr { + require.Nil(t, err) + } + + assert.Equal(t, tt.expectedRetries, retries) + }) + } +}