diff --git a/api/lock.go b/api/lock.go index 0882e520d2..fafe8b5259 100644 --- a/api/lock.go +++ b/api/lock.go @@ -167,6 +167,7 @@ func (l *Lock) Lock(stopCh <-chan struct{}) (<-chan struct{}, error) { WaitTime: l.opts.LockWaitTime, } + start := time.Now() attempts := 0 WAIT: // Check if we should quit @@ -176,9 +177,14 @@ WAIT: default: } - // See if we completed a one-shot. - if attempts > 0 && l.opts.LockTryOnce { - return nil, nil + // Handle the one-shot mode. + if l.opts.LockTryOnce && attempts > 0 { + elapsed := time.Now().Sub(start) + if elapsed > qOpts.WaitTime { + return nil, nil + } + + qOpts.WaitTime -= elapsed } attempts++ diff --git a/api/lock_test.go b/api/lock_test.go index fe279d803c..5cc74e19c3 100644 --- a/api/lock_test.go +++ b/api/lock_test.go @@ -541,8 +541,9 @@ func TestLock_OneShot(t *testing.T) { if ch != nil { t.Fatalf("should not be leader") } - if diff := time.Now().Sub(start); diff > 2*contender.opts.LockWaitTime { - t.Fatalf("took too long: %9.6f", diff.Seconds()) + diff := time.Now().Sub(start) + if diff < contender.opts.LockWaitTime || diff > 2*contender.opts.LockWaitTime { + t.Fatalf("time out of bounds: %9.6f", diff.Seconds()) } // Unlock and then make sure the contender can get it. diff --git a/api/semaphore.go b/api/semaphore.go index ab01773f58..2152ead203 100644 --- a/api/semaphore.go +++ b/api/semaphore.go @@ -186,6 +186,7 @@ func (s *Semaphore) Acquire(stopCh <-chan struct{}) (<-chan struct{}, error) { WaitTime: s.opts.SemaphoreWaitTime, } + start := time.Now() attempts := 0 WAIT: // Check if we should quit @@ -195,9 +196,14 @@ WAIT: default: } - // See if we completed a one-shot. - if attempts > 0 && s.opts.SemaphoreTryOnce { - return nil, nil + // Handle the one-shot mode. + if s.opts.SemaphoreTryOnce && attempts > 0 { + elapsed := time.Now().Sub(start) + if elapsed > qOpts.WaitTime { + return nil, nil + } + + qOpts.WaitTime -= elapsed } attempts++ diff --git a/api/semaphore_test.go b/api/semaphore_test.go index ab296c4beb..4cec164f4e 100644 --- a/api/semaphore_test.go +++ b/api/semaphore_test.go @@ -499,8 +499,9 @@ func TestSemaphore_OneShot(t *testing.T) { if ch != nil { t.Fatalf("should not have acquired the semaphore") } - if diff := time.Now().Sub(start); diff > 2*contender.opts.SemaphoreWaitTime { - t.Fatalf("took too long: %9.6f", diff.Seconds()) + diff := time.Now().Sub(start) + if diff < contender.opts.SemaphoreWaitTime || diff > 2*contender.opts.SemaphoreWaitTime { + t.Fatalf("time out of bounds: %9.6f", diff.Seconds()) } // Give up a slot and make sure the third one can get it. diff --git a/command/lock.go b/command/lock.go index 049d2cbc21..72f4ec5830 100644 --- a/command/lock.go +++ b/command/lock.go @@ -75,8 +75,8 @@ Options: -name="" Optional name to associate with lock session. -token="" ACL token to use. Defaults to that of agent. -pass-stdin Pass stdin to child process. - -try=duration Make a single attempt to acquire the lock, waiting - up to the given duration (eg. "15s"). + -try=timeout Attempt to acquire the lock up to the given + timeout (eg. "15s"). -monitor-retry=n Retry up to n times if Consul returns a 500 error while monitoring the lock. This allows riding out brief periods of unavailability without causing leader @@ -145,12 +145,12 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { var err error wait, err = time.ParseDuration(try) if err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing duration for 'try' option: %s", err)) + c.Ui.Error(fmt.Sprintf("Error parsing try timeout: %s", err)) return 1 } - if wait < 0 { - c.Ui.Error("Duration for 'try' option must be positive") + if wait <= 0 { + c.Ui.Error("Try timeout must be positive") return 1 } diff --git a/command/lock_test.go b/command/lock_test.go index a067f80af5..34c859a51f 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -29,8 +29,9 @@ func argFail(t *testing.T, args []string, expected string) { } func TestLockCommand_BadArgs(t *testing.T) { - argFail(t, []string{"-try=blah", "test/prefix", "date"}, "parsing duration") - argFail(t, []string{"-try=-10s", "test/prefix", "date"}, "must be positive") + argFail(t, []string{"-try=blah", "test/prefix", "date"}, "parsing try timeout") + argFail(t, []string{"-try=0s", "test/prefix", "date"}, "timeout must be positive") + argFail(t, []string{"-try=-10s", "test/prefix", "date"}, "timeout must be positive") argFail(t, []string{"-monitor-retry=-5", "test/prefix", "date"}, "must be >= 0") } diff --git a/website/source/docs/commands/lock.html.markdown b/website/source/docs/commands/lock.html.markdown index dceb4f05f4..cb016412de 100644 --- a/website/source/docs/commands/lock.html.markdown +++ b/website/source/docs/commands/lock.html.markdown @@ -62,10 +62,9 @@ The list of available flags are: * `-pass-stdin` - Pass stdin to child process. -* `-try` - Make a single attempt to acquire the lock, waiting up to the given - duration supplied as the argument. The duration is a decimal number, with - unit suffix, such as "500ms". Valid time units are "ns", "us" (or "µs"), "ms", - "s", "m", "h". +* `-try` - Attempt to acquire the lock up to the given timeout. The timeout is a + positive decimal number, with unit suffix, such as "500ms". Valid time units + are "ns", "us" (or "µs"), "ms", "s", "m", "h". * `-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