Repository: coinbase/rosetta-cli
Pull request #134 (2020-09-15 22:34:33)

[Configuration] Support Max Retries
This PR adds support for specifying a number of `max_retries` instead of using `retry_elapsed_time`. There was a common complaint that `retry_elapsed_time` was hard to reason about with extended timeouts.

### Changes
- [x] add `max_retries` to `configuration`
- [x] change `retry_elapsed_time` to default to 0 so we use `max_retries` by default
- [x] initialize all fetchers with `max_retries`
- [x] update default configuration


Patch:
diff --git a/cmd/check_construction.go b/cmd/check_construction.go
index 01378c5..4b42d18 100644
--- a/cmd/check_construction.go
+++ b/cmd/check_construction.go
@@ -65,6 +65,7 @@ func runCheckConstructionCmd(cmd *cobra.Command, args []string) {
 		fetcher.WithMaxConnections(Config.MaxOnlineConnections),
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	_, _, fetchErr := fetcher.InitializeAsserter(ctx, Config.Network)
diff --git a/cmd/check_data.go b/cmd/check_data.go
index 53c9258..08cefb6 100644
--- a/cmd/check_data.go
+++ b/cmd/check_data.go
@@ -78,6 +78,7 @@ func runCheckDataCmd(cmd *cobra.Command, args []string) {
 		fetcher.WithMaxConnections(Config.MaxOnlineConnections),
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	_, _, fetchErr := fetcher.InitializeAsserter(ctx, Config.Network)
diff --git a/cmd/utils_asserter_configuration.go b/cmd/utils_asserter_configuration.go
index 8b22c05..d92e998 100644
--- a/cmd/utils_asserter_configuration.go
+++ b/cmd/utils_asserter_configuration.go
@@ -49,6 +49,7 @@ func runCreateConfigurationCmd(cmd *cobra.Command, args []string) {
 		Config.OnlineURL,
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	// Initialize the fetcher's asserter
diff --git a/cmd/view_account.go b/cmd/view_account.go
index fb50fa4..55e1da2 100644
--- a/cmd/view_account.go
+++ b/cmd/view_account.go
@@ -63,6 +63,7 @@ func runViewAccountCmd(cmd *cobra.Command, args []string) {
 		Config.OnlineURL,
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	// Initialize the fetcher's asserter
diff --git a/cmd/view_block.go b/cmd/view_block.go
index b0287c3..77f72dd 100644
--- a/cmd/view_block.go
+++ b/cmd/view_block.go
@@ -58,6 +58,7 @@ func runViewBlockCmd(cmd *cobra.Command, args []string) {
 		Config.OnlineURL,
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	// Initialize the fetcher's asserter
diff --git a/cmd/view_networks.go b/cmd/view_networks.go
index 74e2d54..c11c4ff 100644
--- a/cmd/view_networks.go
+++ b/cmd/view_networks.go
@@ -46,6 +46,7 @@ func runViewNetworksCmd(cmd *cobra.Command, args []string) {
 		Config.OnlineURL,
 		fetcher.WithRetryElapsedTime(time.Duration(Config.RetryElapsedTime)*time.Second),
 		fetcher.WithTimeout(time.Duration(Config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(Config.MaxRetries),
 	)
 
 	// Attempt to fetch network list
diff --git a/configuration/configuration.go b/configuration/configuration.go
index 778f927..6598b2d 100644
--- a/configuration/configuration.go
+++ b/configuration/configuration.go
@@ -54,7 +54,7 @@ const (
 const (
 	DefaultURL                               = "http://localhost:8080"
 	DefaultTimeout                           = 10
-	DefaultRetryElapsedTime                  = 60
+	DefaultMaxRetries                        = 5
 	DefaultMaxOnlineConnections              = 120 // most OS have a default limit of 128
 	DefaultMaxOfflineConnections             = 4   // we shouldn't need many connections for construction
 	DefaultMaxSyncConcurrency                = 64
@@ -159,7 +159,7 @@ func DefaultConfiguration() *Configuration {
 		OnlineURL:            DefaultURL,
 		MaxOnlineConnections: DefaultMaxOnlineConnections,
 		HTTPTimeout:          DefaultTimeout,
-		RetryElapsedTime:     DefaultRetryElapsedTime,
+		MaxRetries:           DefaultMaxRetries,
 		MaxSyncConcurrency:   DefaultMaxSyncConcurrency,
 		TipDelay:             DefaultTipDelay,
 		Data:                 DefaultDataConfiguration(),
@@ -293,6 +293,10 @@ type Configuration struct {
 	// HTTPTimeout is the timeout for a HTTP request in seconds.
 	HTTPTimeout uint64 `json:"http_timeout"`
 
+	// MaxRetries is the number of times we will retry an HTTP request. If retry_elapsed_time
+	// is also populated, we may stop attempting retries early.
+	MaxRetries uint64 `json:"max_retries"`
+
 	// RetryElapsedTime is the total time to spend retrying a HTTP request in seconds.
 	RetryElapsedTime uint64 `json:"retry_elapsed_time"`
 
@@ -384,8 +388,8 @@ func populateMissingFields(config *Configuration) *Configuration {
 		config.HTTPTimeout = DefaultTimeout
 	}
 
-	if config.RetryElapsedTime == 0 {
-		config.RetryElapsedTime = DefaultRetryElapsedTime
+	if config.MaxRetries == 0 {
+		config.MaxRetries = DefaultMaxRetries
 	}
 
 	if config.MaxOnlineConnections == 0 {
diff --git a/examples/configuration/default.json b/examples/configuration/default.json
index 7c5c722..e6727ca 100644
--- a/examples/configuration/default.json
+++ b/examples/configuration/default.json
@@ -6,7 +6,8 @@
  "online_url": "http://localhost:8080",
  "data_directory": "",
  "http_timeout": 10,
- "retry_elapsed_time": 60,
+ "max_retries": 5,
+ "retry_elapsed_time": 0,
  "max_online_connections": 120,
  "max_sync_concurrency": 64,
  "tip_delay": 300,


Test patch:
diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go
index 2994a86..30ba581 100644
--- a/configuration/configuration_test.go
+++ b/configuration/configuration_test.go
@@ -51,7 +51,7 @@ var (
 		OnlineURL:            "http://hasudhasjkdk",
 		MaxOnlineConnections: 10,
 		HTTPTimeout:          21,
-		RetryElapsedTime:     1000,
+		MaxRetries:           1000,
 		MaxSyncConcurrency:   12,
 		TipDelay:             1231,
 		Construction: &ConstructionConfiguration{
diff --git a/pkg/tester/construction.go b/pkg/tester/construction.go
index 0bb251d..dd8f0a4 100644
--- a/pkg/tester/construction.go
+++ b/pkg/tester/construction.go
@@ -136,6 +136,7 @@ func InitializeConstruction(
 		fetcher.WithMaxConnections(config.Construction.MaxOfflineConnections),
 		fetcher.WithAsserter(onlineFetcher.Asserter),
 		fetcher.WithTimeout(time.Duration(config.HTTPTimeout)*time.Second),
+		fetcher.WithMaxRetries(config.MaxRetries),
 	)
 
 	// Import prefunded account and save to database