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

v3 api Range continuation is awkward #4385

Closed
krousey opened this issue Feb 2, 2016 · 12 comments
Closed

v3 api Range continuation is awkward #4385

krousey opened this issue Feb 2, 2016 · 12 comments

Comments

@krousey
Copy link

krousey commented Feb 2, 2016

(forked from discussion on kubernetes/kubernetes#20504 (comment))

This could be a complete misunderstanding on my part. But the range API defined here let's me query a half-open range [key, range_end). If I'm trying to continue from a previous range request, I need to populate key correctly. I could use the last key I saw on the previous request [last_seen, end), but that would lead to seeing the last_seen key twice.

It was suggested that I could query [last_seen+1, end) instead. This is awkward because the keys are bytes which translates to all sorts of non-arithmetic types in other languages.

My expectation was that the RangeResponse message would have some sort of opaque continuation token (as in one that I'm not expected to interpret or manipulate) that I could pass to RangeRequest to continue a previous request. This could be implemented by responding with the next key in the range, and I could put that as the start of my next RangeRequest

cc @xiang90 from the other comment thread.

@xiang90
Copy link
Contributor

xiang90 commented Feb 2, 2016

/cc @heyitsanthony

@heyitsanthony
Copy link
Contributor

This seems like something we could support client side on top of the API (RangePager or something). Tracking a token server side would be pricey.

@xiang90
Copy link
Contributor

xiang90 commented Feb 2, 2016

@heyitsanthony The argument is that in some language, byte arithmetic is hard. We probably can just provide an addition field called Next?

@heyitsanthony
Copy link
Contributor

Yeah, that would work. I'm a little hesitant to change the wire protocol to send data that can be computed client side just because some languages have trouble manipulating binary data, though.

@smarterclayton
Copy link
Contributor

Is there a client language affordance function we could add for "next key"? I would assume that would be append(lastSeen, 0x00)?

@xiang90
Copy link
Contributor

xiang90 commented Feb 24, 2016

@krousey Any further comments on this one?

@krousey
Copy link
Author

krousey commented Feb 24, 2016

My only comment would be that leaving it up the client to generate the next key leaves room for them to make the same error I did in describing this bug. I.e incrementing the last byte instead of appending a null byte like @smarterclayton did. Yes, you could provide helper libraries, but do you want to write one for every possible language? Or let the proto compiler generate all the code the clients need?

@xiang90
Copy link
Contributor

xiang90 commented Mar 10, 2016

Or let the proto compiler generate all the code the clients need?

I am confused that the proto complier can generate ALL the code. I think we still need to enable this by writing server side code.

@krousey
Copy link
Author

krousey commented Mar 14, 2016

@xiang90 Yes you would. I simply meant that it would be better if the grpc clients generated by the proto compiler didn't need hand-written helper libraries just to calculate next keys. You could do the calculation once on the server, and every client would not have to have that code.

@xiang90
Copy link
Contributor

xiang90 commented Mar 14, 2016

@krousey I feel we should probably document this in API doc first instead of adding the next field from the very beginning. If any other people start to implement API and feel it is hard or confusing, we can consider to add it directly into server API.

/cc @heyitsanthony Any opinion?

@heyitsanthony
Copy link
Contributor

The next key when paginating a Range request is just lastKey + "\x00" (cf. https://github.com/coreos/etcd/blob/master/clientv3/mirror/syncer.go#L97). It seems somewhat extreme to bake it into the protocol that the server will compute a nul byte append and then send that over the network.

@xiang90
Copy link
Contributor

xiang90 commented Mar 17, 2016

Closing. If there is any real world issue, we shall consider adding a server side hint.

@xiang90 xiang90 closed this as completed Mar 17, 2016
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

4 participants