Resolves #368 - Improve Runbook Performance

.github/workflows/test.yml

@@ -55,7 +55,7 @@ jobs:
           go test -v -cover ./cmd/ ./external/...
 
       - name: Runbook Smoke Test
-        timeout-minutes: 15
+        timeout-minutes: 5
         env:
           EPCC_CLIENT_ID: ${{ secrets.EPCC_CLIENT_ID }}
           EPCC_CLIENT_SECRET: ${{ secrets.EPCC_CLIENT_SECRET }}

.goreleaser.yaml

@@ -38,9 +38,6 @@ nfpms:
     # ID of the nfpm config, must be unique.
     # Defaults to "default".
     package_name: "epcc-cli"
-    replacements:
-      386: i386
-      darwin: macOS
     vendor: Elastic Path Software Inc.
     homepage: https://github.com/elasticpath/epcc-cli
     maintainer: Release Engineering <[email protected]>
@@ -109,4 +106,4 @@ announce:
     #
     # Attention: goreleaser doesn't check the full structure of the Slack API: please make sure that
     # your configuration for advanced message formatting abides by this API.
-    #attachments: []
+    #attachments: []

README.md

@@ -64,7 +64,7 @@ The following is a summary of the main commands, in general you can type `epcc h
 1. `--execution-timeout` will control how long the `epcc` process can run before timing out.
 2. `--rate-limit` will control the number of requests per second to EPCC.
 3. `--max-concurrency` will control the maximum number of concurrent commands that can run simultaneously.
-    * This differs from the rate limit in that if a request takes 2 seconds, a rate limit of 3 will allow 6 requests in flight at a time, where as `--max-concurrency` would limit you to 3. 
+    * This differs from the rate limit in that if a request takes 2 seconds, a rate limit of 3 will allow 6 requests in flight at a time, whereas `--max-concurrency` would limit you to 3. A higher value will slow down initial start time. 
 
 
 ### Configuration

cmd/create.go

@@ -20,7 +20,7 @@ import (
 	"strings"
 )
 
-func NewCreateCommand(parentCmd *cobra.Command) {
+func NewCreateCommand(parentCmd *cobra.Command) func() {
 
 	var createCmd = &cobra.Command{
 		Use:   "create",
@@ -34,16 +34,28 @@ func NewCreateCommand(parentCmd *cobra.Command) {
 		},
 	}
 
+	overrides := &httpclient.HttpParameterOverrides{
+		QueryParameters: nil,
+		OverrideUrlPath: "",
+	}
+
+	// Ensure that any new options here are added to the resetFunc
 	var autoFillOnCreate = false
 	var noBodyPrint = false
 	var outputJq = ""
 	var setAlias = ""
 	var ifAliasExists = ""
 	var ifAliasDoesNotExist = ""
 
-	overrides := &httpclient.HttpParameterOverrides{
-		QueryParameters: nil,
-		OverrideUrlPath: "",
+	resetFunc := func() {
+		autoFillOnCreate = false
+		noBodyPrint = false
+		outputJq = ""
+		setAlias = ""
+		ifAliasExists = ""
+		ifAliasDoesNotExist = ""
+		overrides.OverrideUrlPath = ""
+		overrides.QueryParameters = nil
 	}
 
 	for _, resource := range resources.GetPluralResources() {
@@ -198,6 +210,8 @@ func NewCreateCommand(parentCmd *cobra.Command) {
 	createCmd.PersistentFlags().StringVarP(&ifAliasDoesNotExist, "if-alias-does-not-exist", "", "", "If the alias does not exist we will run this command, otherwise exit with no error")
 	createCmd.MarkFlagsMutuallyExclusive("if-alias-exists", "if-alias-does-not-exist")
 	_ = createCmd.RegisterFlagCompletionFunc("output-jq", jqCompletionFunc)
+
+	return resetFunc
 }
 
 func createInternal(ctx context.Context, overrides *httpclient.HttpParameterOverrides, args []string, autoFillOnCreate bool, aliasName string) (string, error) {
@@ -313,6 +327,7 @@ func createInternal(ctx context.Context, overrides *httpclient.HttpParameterOver
 		if aliasName != "" {
 			aliases.SetAliasForResource(string(resBody), aliasName)
 		}
+
 		return string(resBody), nil
 	} else {
 		return "", nil

cmd/delete-all.go

@@ -17,7 +17,7 @@ import (
 	"sync"
 )
 
-func NewDeleteAllCommand(parentCmd *cobra.Command) {
+func NewDeleteAllCommand(parentCmd *cobra.Command) func() {
 
 	var deleteAll = &cobra.Command{
 		Use:          "delete-all",
@@ -53,6 +53,7 @@ func NewDeleteAllCommand(parentCmd *cobra.Command) {
 		deleteAll.AddCommand(deleteAllResourceCmd)
 	}
 	parentCmd.AddCommand(deleteAll)
+	return func() {}
 
 }
 func deleteAllInternal(ctx context.Context, args []string) error {

cmd/delete.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 )
 
-func NewDeleteCommand(parentCmd *cobra.Command) {
+func NewDeleteCommand(parentCmd *cobra.Command) func() {
 
 	var deleteCmd = &cobra.Command{
 		Use:          "delete",
@@ -37,11 +37,19 @@ func NewDeleteCommand(parentCmd *cobra.Command) {
 		OverrideUrlPath: "",
 	}
 
+	// Ensure that any new options here are added to the resetFunc
 	var allow404 = false
-
 	var ifAliasExists = ""
 	var ifAliasDoesNotExist = ""
 
+	resetFunc := func() {
+		overrides.QueryParameters = nil
+		overrides.OverrideUrlPath = ""
+		allow404 = false
+		ifAliasExists = ""
+		ifAliasDoesNotExist = ""
+	}
+
 	for _, resource := range resources.GetPluralResources() {
 		if resource.DeleteEntityInfo == nil {
 			continue
@@ -159,6 +167,8 @@ func NewDeleteCommand(parentCmd *cobra.Command) {
 	deleteCmd.PersistentFlags().StringVarP(&ifAliasDoesNotExist, "if-alias-does-not-exist", "", "", "If the alias does not exist we will run this command, otherwise exit with no error")
 	deleteCmd.MarkFlagsMutuallyExclusive("if-alias-exists", "if-alias-does-not-exist")
 	parentCmd.AddCommand(deleteCmd)
+
+	return resetFunc
 }
 func deleteInternal(ctx context.Context, overrides *httpclient.HttpParameterOverrides, allow404 bool, args []string) (string, error) {
 	crud.OutstandingRequestCounter.Add(1)

cmd/get.go

@@ -22,22 +22,31 @@ import (
 const singularResourceRequest = 0
 const collectionResourceRequest = 1
 
-func NewGetCommand(parentCmd *cobra.Command) {
+func NewGetCommand(parentCmd *cobra.Command) func() {
 	overrides := &httpclient.HttpParameterOverrides{
 		QueryParameters: nil,
 		OverrideUrlPath: "",
 	}
 
+	// Ensure that any new options here are added to the resetFunc
 	var outputJq = ""
 	var noBodyPrint = false
-
 	var retryWhileJQ = ""
-
 	var retryWhileJQMaxAttempts = uint16(1200)
-
 	var ifAliasExists = ""
 	var ifAliasDoesNotExist = ""
 
+	resetFunc := func() {
+		overrides.QueryParameters = nil
+		overrides.OverrideUrlPath = ""
+		outputJq = ""
+		noBodyPrint = false
+		retryWhileJQ = ""
+		retryWhileJQMaxAttempts = uint16(1200)
+		ifAliasExists = ""
+		ifAliasDoesNotExist = ""
+	}
+
 	var getCmd = &cobra.Command{
 		Use:          "get",
 		Short:        "Retrieves either a single or all resources",
@@ -263,6 +272,8 @@ func NewGetCommand(parentCmd *cobra.Command) {
 	_ = getCmd.RegisterFlagCompletionFunc("output-jq", jqCompletionFunc)
 
 	parentCmd.AddCommand(getCmd)
+
+	return resetFunc
 }
 
 func getInternal(ctx context.Context, overrides *httpclient.HttpParameterOverrides, args []string) (string, error) {

cmd/root.go

@@ -249,8 +249,10 @@ func Execute() {
 
 	if err != nil {
 		log.Errorf("Error occurred while processing command: %s", err)
+
 		os.Exit(1)
 	} else {
+
 		os.Exit(0)
 	}
 }

cmd/runbooks.go

@@ -9,6 +9,7 @@ import (
 	"github.com/elasticpath/epcc-cli/external/runbooks"
 	_ "github.com/elasticpath/epcc-cli/external/runbooks"
 	"github.com/elasticpath/epcc-cli/external/shutdown"
+	"github.com/jolestar/go-commons-pool/v2"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/semaphore"
@@ -113,7 +114,7 @@ func initRunbookRunCommands() *cobra.Command {
 	}
 
 	execTimeoutInSeconds := runbookRunCommand.PersistentFlags().Int64("execution-timeout", 900, "How long should the script take to execute before timing out")
-	maxConcurrency := runbookRunCommand.PersistentFlags().Int64("max-concurrency", 2048, "Maximum number of commands at once")
+	maxConcurrency := runbookRunCommand.PersistentFlags().Int("max-concurrency", 20, "Maximum number of commands that can run simultaneously")
 
 	for _, runbook := range runbooks.GetRunbooks() {
 		// Create a copy of runbook scoped to the loop
@@ -146,8 +147,16 @@ func initRunbookRunCommands() *cobra.Command {
 
 					ctx, cancelFunc := context.WithCancel(parentCtx)
 
-					concurrentRunSemaphore := semaphore.NewWeighted(*maxConcurrency)
+					concurrentRunSemaphore := semaphore.NewWeighted(int64(*maxConcurrency))
+					factory := pool.NewPooledObjectFactorySimple(
+						func(ctx2 context.Context) (interface{}, error) {
+							return generateRunbookCmd(), nil
+						})
 
+					objectPool := pool.NewObjectPool(ctx, factory, &pool.ObjectPoolConfig{
+						MaxTotal: *maxConcurrency,
+						MaxIdle:  *maxConcurrency,
+					})
 					for stepIdx, rawCmd := range runbookAction.RawCommands {
 
 						// Create a copy of loop variables
@@ -191,11 +200,22 @@ func initRunbookRunCommands() *cobra.Command {
 
 								log.Tracef("(Step %d/%d Command %d/%d) Building Commmand", stepIdx+1, numSteps, commandIdx+1, len(funcs))
 
-								stepCmd := generateRunbookCmd()
-								stepCmd.SetArgs(rawCmdArguments[1:])
-								log.Tracef("(Step %d/%d Command %d/%d) Starting Command", stepIdx+1, numSteps, commandIdx+1, len(funcs))
-								err := stepCmd.ExecuteContext(ctx)
-								log.Tracef("(Step %d/%d Command %d/%d) Complete Command", stepIdx+1, numSteps, commandIdx+1, len(funcs))
+								stepCmdObject, err := objectPool.BorrowObject(ctx)
+								defer objectPool.ReturnObject(ctx, stepCmdObject)
+
+								if err == nil {
+									commandAndResetFunc := stepCmdObject.(*CommandAndReset)
+									commandAndResetFunc.reset()
+									stepCmd := commandAndResetFunc.cmd
+
+									stepCmd.SetArgs(rawCmdArguments[1:])
+									log.Tracef("(Step %d/%d Command %d/%d) Starting Command", stepIdx+1, numSteps, commandIdx+1, len(funcs))
+
+									stepCmd.ResetFlags()
+									err = stepCmd.ExecuteContext(ctx)
+									log.Tracef("(Step %d/%d Command %d/%d) Complete Command", stepIdx+1, numSteps, commandIdx+1, len(funcs))
+								}
+
 								commandResult := &commandResult{
 									stepIdx:     stepIdx,
 									commandIdx:  commandIdx,
@@ -314,20 +334,36 @@ func processRunbookVariablesOnCommand(runbookActionRunActionCommand *cobra.Comma
 
 // Creates a new instance of a cobra.Command
 // We use a new instance for each step so that we can benefit from flags in runbooks
-func generateRunbookCmd() *cobra.Command {
+
+type CommandAndReset struct {
+	cmd   *cobra.Command
+	reset func()
+}
+
+func generateRunbookCmd() *CommandAndReset {
 	root := &cobra.Command{
 		Use:          "epcc",
 		SilenceUsage: true,
 	}
 
-	NewCreateCommand(root)
-	NewUpdateCommand(root)
-	NewDeleteCommand(root)
-	NewGetCommand(root)
-	NewDeleteAllCommand(root)
+	resetCreateCmd := NewCreateCommand(root)
+	resetUpdateCmd := NewUpdateCommand(root)
+	resetDeleteCmd := NewDeleteCommand(root)
+	resetGetCmd := NewGetCommand(root)
+	resetDeleteAllCmd := NewDeleteAllCommand(root)
 	getDevCommands(root)
 
-	return root
+	return &CommandAndReset{
+		root,
+		func() {
+			// We need to reset the state of all commands since we are reusing the objects
+			resetCreateCmd()
+			resetUpdateCmd()
+			resetDeleteCmd()
+			resetGetCmd()
+			resetDeleteAllCmd()
+		},
+	}
 }
 
 func initRunbookDevCommands() *cobra.Command {

cmd/update.go

@@ -17,19 +17,27 @@ import (
 	"strings"
 )
 
-func NewUpdateCommand(parentCmd *cobra.Command) {
+func NewUpdateCommand(parentCmd *cobra.Command) func() {
 	overrides := &httpclient.HttpParameterOverrides{
 		QueryParameters: nil,
 		OverrideUrlPath: "",
 	}
 
+	// Ensure that any new options here are added to the resetFunc
 	var outputJq = ""
-
 	var noBodyPrint = false
-
 	var ifAliasExists = ""
 	var ifAliasDoesNotExist = ""
 
+	resetFunc := func() {
+		overrides.QueryParameters = nil
+		overrides.OverrideUrlPath = ""
+		outputJq = ""
+		noBodyPrint = false
+		ifAliasExists = ""
+		ifAliasDoesNotExist = ""
+	}
+
 	var updateCmd = &cobra.Command{
 		Use:          "update",
 		Short:        "Updates a resource",
@@ -175,6 +183,8 @@ func NewUpdateCommand(parentCmd *cobra.Command) {
 	_ = updateCmd.RegisterFlagCompletionFunc("output-jq", jqCompletionFunc)
 	parentCmd.AddCommand(updateCmd)
 
+	return resetFunc
+
 }
 
 func updateInternal(ctx context.Context, overrides *httpclient.HttpParameterOverrides, args []string) (string, error) {

external/httpclient/httpclient.go

@@ -112,7 +112,11 @@ func LogStats() {
 	counts := ""
 
 	for _, k := range keys {
-		counts += fmt.Sprintf("%d:%d, ", k, stats.respCodes[k])
+		if k == 0 {
+			counts += fmt.Sprintf("%d:%d, ", k, stats.respCodes[k])
+		} else {
+			counts += fmt.Sprintf("CONN_ERROR:%d, ", stats.respCodes[k])
+		}
 	}
 
 	if stats.totalRequests > 3 {
@@ -225,7 +229,7 @@ func doRequestInternal(ctx context.Context, method string, contentType string, p
 
 	log.Tracef("Waiting for rate limiter")
 	if err := Limit.Wait(ctx); err != nil {
-		return nil, fmt.Errorf("Rate limiter returned error %v, %w", err, err)
+		return nil, fmt.Errorf("rate limiter returned error %v, %w", err, err)
 	}
 
 	rateLimitTime := time.Since(start)
@@ -237,13 +241,15 @@ func doRequestInternal(ctx context.Context, method string, contentType string, p
 	stats.totalRequests += 1
 	if rateLimitTime.Milliseconds() > 50 {
 		// Only count rate limit time if it took us longer than 50 ms to get here.
-		stats.totalRateLimitedTimeInMs += int64(rateLimitTime.Milliseconds())
+		stats.totalRateLimitedTimeInMs += rateLimitTime.Milliseconds()
 	}
 
-	stats.totalHttpRequestProcessingTime += int64(requestTime.Milliseconds()) - int64(rateLimitTime.Milliseconds())
+	stats.totalHttpRequestProcessingTime += requestTime.Milliseconds() - rateLimitTime.Milliseconds()
 
 	if resp != nil {
 		stats.respCodes[resp.StatusCode] = stats.respCodes[resp.StatusCode] + 1
+	} else {
+		stats.respCodes[0] = stats.respCodes[0] + 1
 	}
 
 	requestNumber := stats.totalRequests