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

Simultaneous acquisition of kv session lock and atomic update succeeds regardless of modifyIndex #432

Closed
tcollinsworth opened this issue Oct 26, 2014 · 1 comment · Fixed by #634
Labels
type/docs Documentation needs to be created/updated/clarified

Comments

@tcollinsworth
Copy link

PUT /v1/kv/someKey?cas=11789&acquire=b2d5847c-08c2-e8e6-d5b4-18d419e1d8e5

Always updates the value and returns true even if the the modifyIndex was incremented by another change (i.e., via the UI) after reading the modifyIndex and before writing with the old lower modifyIndex. When the acquire=sessionId is removed, the same code properly fails to update the value and receives false.

Can you not combine query string modifiers? Didn't see this in the documentation.

Was trying to use this for leadership management. Ideally it would all succeed or all fail. Without an all succeed or all failure semantic, since the lock is advisory, it requires more round trips.

1 acquire lock get back sessionId
2 query key to validate sessionId is still current and to get current modifyIndex, stop if sessionId does not match
3 update with cas, if session was lost update will fail because it incremented modifyIndex

The value being updated is to set the current clientId into the value of the lock Key so the leader will be able to detect when it successfully consecutively reacquires a lock after a lock loss due to false positive failure detection.

@armon
Copy link
Member

armon commented Oct 27, 2014

The ?cas= and ?acquire= operations are distinct and cannot be composed in this manner. I'm not sure which of the two you are getting since it's not well defined, but you don't get the semantics of both of them.

@ryanuber ryanuber added the type/docs Documentation needs to be created/updated/clarified label Nov 26, 2014
duckhan pushed a commit to duckhan/consul that referenced this issue Oct 24, 2021
This refactor paves the way for adding the cleanup controller. It
removes the if/else statement around starting the mutating webhook
server when health checks are enabled vs when they're not and instead
starts it the same way in both cases.

It also removes special validation around the CONSUL_HTTP_ADDR because
this was already being discovered earlier in the file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants