Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semaphore Acquire does not respect DefaultSemaphoreRetryTime #932

Closed
highlyunavailable opened this issue May 12, 2015 · 3 comments · Fixed by #941
Closed

Semaphore Acquire does not respect DefaultSemaphoreRetryTime #932

highlyunavailable opened this issue May 12, 2015 · 3 comments · Fixed by #941

Comments

@highlyunavailable
Copy link
Contributor

https://github.com/hashicorp/consul/blob/master/api/semaphore.go#L237

The DefaultSemaphoreRetryTime argument is not used during the wait if the semaphore is not acquired, unlike the lock variant:

https://github.com/hashicorp/consul/blob/master/api/lock.go#L187

@armon
Copy link
Member

armon commented May 12, 2015

@highlyunavailable I think this is okay actually. Lock makes use of the ?acquire operation which may be affected by a lock-delay, while semaphore uses ?cas which does not have the same sort of deferral mechanism.

@highlyunavailable
Copy link
Contributor Author

Fair enough. Does it make sense to remove DefaultSemaphoreRetryTime from the code then, since it isn't used anywhere?

@armon
Copy link
Member

armon commented May 12, 2015

Probably, I think that is a copy/paste artifact

highlyunavailable added a commit to highlyunavailable/consul that referenced this issue May 15, 2015
Fixes hashicorp#932

DefaultSemaphoreRetryTime is actually unused, unlike DefaultLockRetryTime.
duckhan pushed a commit to duckhan/consul that referenced this issue Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants