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

Adding LeaseID checking in Txn needs a Cmp type method #8487

Closed
purpleidea opened this issue Sep 1, 2017 · 7 comments
Closed

Adding LeaseID checking in Txn needs a Cmp type method #8487

purpleidea opened this issue Sep 1, 2017 · 7 comments

Comments

@purpleidea
Copy link
Contributor

I'd like to do Txn's like this:

If v3.Compare(v3.LeaseValue(keyPath), "=", someLeaseID) ...

Thanks to some lovely work @heyitsanthony pointed me to, you could write:

	LeaseValue := func(key string) v3.Cmp {
		return v3.Cmp{Key: []byte(key), Target: pb.Compare_LEASE}
	}

I'd propose adding this as a helper function alongside https://godoc.org/github.com/coreos/etcd/clientv3#Value but there are apparently some naming convention issues in "mapping directly to the mvccpb.KeyValue".

These are beyond my understanding of the internals, but this issue is to track this so interested users get a notification when this is fixed so we can update our code.

Thanks for the great feature!

@purpleidea
Copy link
Contributor Author

purpleidea commented Sep 1, 2017

I wrote a patch adding this if anyone would like it.
This is nice to avoid having the end user import pb and know that this trick is possible.

#8488

@davissp14
Copy link
Contributor

https://github.com/coreos/etcd/blob/master/etcdserver/etcdserverpb/rpc.proto#L520

This feature doesn't seem to be working as expected. I am working on adding support for this here: https://github.com/davissp14/etcdv3-ruby/pull/95/files

The txn lease comparison seems to ALWAYS return successfully regardless of the comparison. Is this issue relevant here? If not, let me know and I can open up a new issue.

@xiang90
Copy link
Contributor

xiang90 commented Sep 3, 2017

@davissp14 what is the server version? the feature only works on master.

@davissp14
Copy link
Contributor

@xiang90 That would explain it. The server version I am testing against is 3.2.6.

@heyitsanthony
Copy link
Contributor

Fixed in master. Closing.

@danevandyck
Copy link

Have you considered adding comparison on lease ID to 3.2?

@gyuho
Copy link
Contributor

gyuho commented Oct 17, 2017

@danevandyck This was fixed via #8324.
And will be included in our upcoming 3.3 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants