fix Compare and swap returns incorrect cause #614
Conversation
| @@ -322,10 +322,15 @@ func (n *node) UpdateTTL(expireTime time.Time) { | |||
| } | |||
|
|
|||
| func (n *node) Compare(prevValue string, prevIndex uint64) bool { | |||
There was a problem hiding this comment.
@metaflow Can you change this to return (ok bool, which int).
So you do not need to compare it again in getCompareFailCause.
There was a problem hiding this comment.
@xiangli-cmu sure
for for both match which = 0
only index match - 1
only value match - 2
both mismatch - 3
sounds good?
There was a problem hiding this comment.
Can you add const variables for these const numbers? Like
const (
match = 0
indexNotMatch = 1
valueNotMatch = 2
NoMatch =3
)
I think we can just use positive number. 1 for index not match, 2 for value not match, and 3 for neither of them.
There was a problem hiding this comment.
@xiangli-cmu one more question - it should be added as exported const in header of this file, shouldn't it?
There was a problem hiding this comment.
@metaflow Oh. Yes. since the function is exported.
|
@metaflow Thank you for working on this! |
|
@xiangli-cmu I have changed result of |
|
lgtm |
|
Can you squash these commits? |
specifically when both prevIndex and prevValue are provided
|
@xiangli-cmu @philips done, fixed commit message as well |
|
I will update API docs in another PR if you don't mind (I already have #610 not merged yet :) so I can add this there) |
|
lgtm, merging. |
fix Compare and swap returns incorrect cause
fix for issue #611
Note that compare-and-delete cause changed too.
I worry about modifications in v1 tests while v1 API is frozen.
Thank you!