From 1d733f4c36d787e2ee634543d9b27021afa66235 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Tue, 5 Jan 2016 18:34:22 -0800 Subject: [PATCH] Defaults monitor retries to 3 retries @ 1s for the consul lock command. --- command/lock.go | 29 +++++-- command/lock_test.go | 84 +++++++++++++++++-- .../source/docs/commands/lock.html.markdown | 3 +- 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/command/lock.go b/command/lock.go index 55ec9c5cc0..049d2cbc21 100644 --- a/command/lock.go +++ b/command/lock.go @@ -24,6 +24,14 @@ const ( // lock-delay value of 15 seconds. This only affects locks and not // semaphores. lockKillGracePeriod = 5 * time.Second + + // defaultMonitorRetry is the number of 500 errors we will tolerate + // before declaring the lock gone. + defaultMonitorRetry = 3 + + // defaultMonitorRetryTime is the amount of time to wait between + // retries. + defaultMonitorRetryTime = 1 * time.Second ) // LockCommand is a Command implementation that is used to setup @@ -73,7 +81,8 @@ Options: while monitoring the lock. This allows riding out brief periods of unavailability without causing leader elections, but increases the amount of time required - to detect a lost lock in some cases. Defaults to 0. + to detect a lost lock in some cases. Defaults to 3, + with a 1s wait between retries. Set to 0 to disable. -verbose Enables verbose output ` return strings.TrimSpace(helpText) @@ -99,7 +108,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { cmdFlags.StringVar(&token, "token", "", "") cmdFlags.BoolVar(&passStdin, "pass-stdin", false, "") cmdFlags.StringVar(&try, "try", "", "") - cmdFlags.IntVar(&retry, "monitor-retry", 0, "") + cmdFlags.IntVar(&retry, "monitor-retry", defaultMonitorRetry, "") cmdFlags.BoolVar(&c.verbose, "verbose", false, "") httpAddr := HTTPAddrFlag(cmdFlags) if err := cmdFlags.Parse(args); err != nil { @@ -271,9 +280,10 @@ func (c *LockCommand) setupLock(client *api.Client, prefix, name string, c.Ui.Info(fmt.Sprintf("Setting up lock at path: %s", key)) } opts := api.LockOptions{ - Key: key, - SessionName: name, - MonitorRetries: retry, + Key: key, + SessionName: name, + MonitorRetries: retry, + MonitorRetryTime: defaultMonitorRetryTime, } if oneshot { opts.LockTryOnce = true @@ -304,10 +314,11 @@ func (c *LockCommand) setupSemaphore(client *api.Client, limit int, prefix, name c.Ui.Info(fmt.Sprintf("Setting up semaphore (limit %d) at prefix: %s", limit, prefix)) } opts := api.SemaphoreOptions{ - Prefix: prefix, - Limit: limit, - SessionName: name, - MonitorRetries: retry, + Prefix: prefix, + Limit: limit, + SessionName: name, + MonitorRetries: retry, + MonitorRetryTime: defaultMonitorRetryTime, } if oneshot { opts.SemaphoreTryOnce = true diff --git a/command/lock_test.go b/command/lock_test.go index f5f84ef9dd..a067f80af5 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -121,7 +121,7 @@ func TestLockCommand_Try_Semaphore(t *testing.T) { } } -func TestLockCommand_MonitorRetry_Lock(t *testing.T) { +func TestLockCommand_MonitorRetry_Lock_Default(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() waitForLeader(t, a1.httpAddr) @@ -130,7 +130,7 @@ func TestLockCommand_MonitorRetry_Lock(t *testing.T) { c := &LockCommand{Ui: ui} filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a1.httpAddr, "-monitor-retry=3", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a1.httpAddr, "test/prefix", touchCmd} // Run the command. var lu *LockUnlock @@ -148,12 +148,13 @@ func TestLockCommand_MonitorRetry_Lock(t *testing.T) { if !ok { t.Fatalf("bad type") } - if opts.MonitorRetries != 3 { - t.Fatalf("bad: %d", opts.MonitorRetries) + if opts.MonitorRetries != defaultMonitorRetry || + opts.MonitorRetryTime != defaultMonitorRetryTime { + t.Fatalf("bad: %#v", opts) } } -func TestLockCommand_MonitorRetry_Semaphore(t *testing.T) { +func TestLockCommand_MonitorRetry_Semaphore_Default(t *testing.T) { a1 := testAgent(t) defer a1.Shutdown() waitForLeader(t, a1.httpAddr) @@ -162,7 +163,7 @@ func TestLockCommand_MonitorRetry_Semaphore(t *testing.T) { c := &LockCommand{Ui: ui} filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) - args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "-monitor-retry=3", "test/prefix", touchCmd} + args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "test/prefix", touchCmd} // Run the command. var lu *LockUnlock @@ -180,7 +181,74 @@ func TestLockCommand_MonitorRetry_Semaphore(t *testing.T) { if !ok { t.Fatalf("bad type") } - if opts.MonitorRetries != 3 { - t.Fatalf("bad: %d", opts.MonitorRetries) + if opts.MonitorRetries != defaultMonitorRetry || + opts.MonitorRetryTime != defaultMonitorRetryTime { + t.Fatalf("bad: %#v", opts) + } +} + +func TestLockCommand_MonitorRetry_Lock_Arg(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + waitForLeader(t, a1.httpAddr) + + ui := new(cli.MockUi) + c := &LockCommand{Ui: ui} + filePath := filepath.Join(a1.dir, "test_touch") + touchCmd := fmt.Sprintf("touch '%s'", filePath) + args := []string{"-http-addr=" + a1.httpAddr, "-monitor-retry=9", "test/prefix", touchCmd} + + // Run the command. + var lu *LockUnlock + code := c.run(args, &lu) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + _, err := ioutil.ReadFile(filePath) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Make sure the monitor options were set correctly. + opts, ok := lu.rawOpts.(*api.LockOptions) + if !ok { + t.Fatalf("bad type") + } + if opts.MonitorRetries != 9 || + opts.MonitorRetryTime != defaultMonitorRetryTime { + t.Fatalf("bad: %#v", opts) + } +} + +func TestLockCommand_MonitorRetry_Semaphore_Arg(t *testing.T) { + a1 := testAgent(t) + defer a1.Shutdown() + waitForLeader(t, a1.httpAddr) + + ui := new(cli.MockUi) + c := &LockCommand{Ui: ui} + filePath := filepath.Join(a1.dir, "test_touch") + touchCmd := fmt.Sprintf("touch '%s'", filePath) + args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "-monitor-retry=9", "test/prefix", touchCmd} + + // Run the command. + var lu *LockUnlock + code := c.run(args, &lu) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + _, err := ioutil.ReadFile(filePath) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Make sure the monitor options were set correctly. + opts, ok := lu.rawOpts.(*api.SemaphoreOptions) + if !ok { + t.Fatalf("bad type") + } + if opts.MonitorRetries != 9 || + opts.MonitorRetryTime != defaultMonitorRetryTime { + t.Fatalf("bad: %#v", opts) } } diff --git a/website/source/docs/commands/lock.html.markdown b/website/source/docs/commands/lock.html.markdown index fe6f49f219..dceb4f05f4 100644 --- a/website/source/docs/commands/lock.html.markdown +++ b/website/source/docs/commands/lock.html.markdown @@ -70,7 +70,8 @@ The list of available flags are: * `-monitor-retry` - Retry up to this number of times if Consul returns a 500 error while monitoring the lock. This allows riding out brief periods of unavailability without causing leader elections, but increases the amount of time required - to detect a lost lock in some cases. Defaults to 0. + to detect a lost lock in some cases. Defaults to 3, with a 1s wait between retries. + Set to 0 to disable. * `-verbose` - Enables verbose output.