From 78339e78115ee1453e26ec7fd7a3d04c49f3c1a2 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Tue, 13 Dec 2016 13:06:58 -0700 Subject: [PATCH] Teach manifest how to configure MaxRetries --- tq/manifest.go | 14 ++++++++++++ tq/transfer_queue.go | 25 +++++--------------- tq/transfer_queue_test.go | 48 ++++++++++++++++++++------------------- 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/tq/manifest.go b/tq/manifest.go index be3e840a..967037a7 100644 --- a/tq/manifest.go +++ b/tq/manifest.go @@ -8,6 +8,10 @@ import ( ) type Manifest struct { + // MaxRetries is the maximum number of retries a single object can + // attempt to make before it will be dropped. + MaxRetries int `git:"lfs.transfer.maxretries"` + basicTransfersOnly bool downloadAdapterFuncs map[string]NewAdapterFunc uploadAdapterFuncs map[string]NewAdapterFunc @@ -22,6 +26,16 @@ func NewManifest() *Manifest { } func ConfigureManifest(m *Manifest, cfg *config.Configuration) *Manifest { + if err := cfg.Unmarshal(m); err != nil { + tracerx.Printf("manifest: error parsing config, falling back to default values...: %v", err) + m.MaxRetries = 1 + } + + if m.MaxRetries < 1 { + tracerx.Printf("manifest: invalid retry count: %d, defaulting to %d", m.MaxRetries, 1) + m.MaxRetries = 1 + } + m.basicTransfersOnly = cfg.BasicTransfersOnly() configureBasicDownloadAdapter(m) diff --git a/tq/transfer_queue.go b/tq/transfer_queue.go index 4e1c9eb2..f4798914 100644 --- a/tq/transfer_queue.go +++ b/tq/transfer_queue.go @@ -26,8 +26,6 @@ type Transferable interface { } type retryCounter struct { - // MaxRetries is the maximum number of retries a single object can - // attempt to make before it will be dropped. MaxRetries int `git:"lfs.transfer.maxretries"` // cmu guards count @@ -42,24 +40,11 @@ type retryCounter struct { // // If it encountered an error in Unmarshaling the *config.Configuration, it will // be returned, otherwise nil. -func newRetryCounter(cfg *config.Configuration) *retryCounter { - rc := &retryCounter{ +func newRetryCounter() *retryCounter { + return &retryCounter{ MaxRetries: defaultMaxRetries, - - count: make(map[string]int), + count: make(map[string]int), } - - if err := cfg.Unmarshal(rc); err != nil { - tracerx.Printf("rc: error parsing config, falling back to default values...: %v", err) - rc.MaxRetries = 1 - } - - if rc.MaxRetries < 1 { - tracerx.Printf("rc: invalid retry count: %d, defaulting to %d", rc.MaxRetries, 1) - rc.MaxRetries = 1 - } - - return rc } // Increment increments the number of retries for a given OID. It is safe to @@ -173,13 +158,15 @@ func NewTransferQueue(dir Direction, options ...Option) *TransferQueue { transferables: make(map[string]Transferable), trMutex: &sync.Mutex{}, manifest: ConfigureManifest(NewManifest(), config.Config), - rc: newRetryCounter(config.Config), + rc: newRetryCounter(), } for _, opt := range options { opt(q) } + q.rc.MaxRetries = q.manifest.MaxRetries + if q.batchSize <= 0 { q.batchSize = defaultBatchSize } diff --git a/tq/transfer_queue_test.go b/tq/transfer_queue_test.go index 87bf95f5..e372991f 100644 --- a/tq/transfer_queue_test.go +++ b/tq/transfer_queue_test.go @@ -7,56 +7,58 @@ import ( "github.com/stretchr/testify/assert" ) -func TestRetryCounterDefaultsToFixedRetries(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{})) - - assert.Equal(t, 1, rc.MaxRetries) +func TestManifestDefaultsToFixedRetries(t *testing.T) { + cfg := config.NewFrom(config.Values{}) + m := ConfigureManifest(NewManifest(), cfg) + assert.Equal(t, 1, m.MaxRetries) } -func TestRetryCounterIsConfigurable(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{ +func TestManifestIsConfigurable(t *testing.T) { + cfg := config.NewFrom(config.Values{ Git: map[string]string{ "lfs.transfer.maxretries": "3", }, - })) - - assert.Equal(t, 3, rc.MaxRetries) + }) + m := ConfigureManifest(NewManifest(), cfg) + assert.Equal(t, 3, m.MaxRetries) } -func TestRetryCounterClampsValidValues(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{ +func TestManifestClampsValidValues(t *testing.T) { + cfg := config.NewFrom(config.Values{ Git: map[string]string{ "lfs.transfer.maxretries": "-1", }, - })) - - assert.Equal(t, 1, rc.MaxRetries) + }) + m := ConfigureManifest(NewManifest(), cfg) + assert.Equal(t, 1, m.MaxRetries) } -func TestRetryCounterIgnoresNonInts(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{ +func TestManifestIgnoresNonInts(t *testing.T) { + cfg := config.NewFrom(config.Values{ Git: map[string]string{ "lfs.transfer.maxretries": "not_an_int", }, - })) + }) + m := ConfigureManifest(NewManifest(), cfg) + assert.Equal(t, 1, m.MaxRetries) +} +func TestRetryCounterDefaultsToFixedRetries(t *testing.T) { + rc := newRetryCounter() assert.Equal(t, 1, rc.MaxRetries) } func TestRetryCounterIncrementsObjects(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{})) - + rc := newRetryCounter() rc.Increment("oid") - assert.Equal(t, 1, rc.CountFor("oid")) } func TestRetryCounterCanNotRetryAfterExceedingRetryCount(t *testing.T) { - rc := newRetryCounter(config.NewFrom(config.Values{})) - + rc := newRetryCounter() rc.Increment("oid") - count, canRetry := rc.CanRetry("oid") + count, canRetry := rc.CanRetry("oid") assert.Equal(t, 1, count) assert.False(t, canRetry) }