-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Optimize RangeRequest Revision count #16510
Comments
would like your opinion on this |
Kubernetes depends on count, so doesn't look like this feature will get much use. |
Making the condition being false (e.g. no sorting, etc.) can get the same better performance? @mehvaibh |
Yes, API Server uses If we implement this feature, API Server can also take advantage of this. But here, I wanted to check feasibility from ETCD POV. |
Not sure I understand this correctly but seems like revisions will loop on to calculate total even if limit is less than total. Pls correct me if I am wrong. |
In your example, you set the limit to 2. If you change it to a larger limit (e.g. 1K), I think the performance difference may be negligible (can you double check that?). In large range query, the bottleneck should be reading data from the storage engine instead of counting the total number in memory. Adding the |
I think Kubernetes would benefit much more from serving pagination from watch cache instead of trying to optimise patination on etcd side, but I'm biased by my own work https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2340-Consistent-reads-from-cache. cc @mborsz |
time ./etcdctl get --prefix /registry/configmaps/default --limit 2000 > /dev/null
@ahrtr I tried with 2K page limit. The optimized server(respects limit) latency remains the same while non-optimized server(current ETCD) latencies increases with number of objects.
Agree. Sort range queries overrides the This optimization can benefit no sort paginated range request. |
@serathius Thanks for the update! I think this optimization can also help reduce paginated no sort range queries latencies specially when number of objects are high. As ETCD needs to spend relatively less time completing paginated no sort range transactions it will be helpful in large DB with high number of objects count and under significant load (i.e. varying mix of read/write transactions). |
thx for the feedback. I am not against it as long as the corresponding K8s's proposal can be accepted [in other words, as long as there are valid use cases]. |
This proposal looks like Most of times, the client should get result from apiserver's cache. But the client can still force query to etcd cluster since the kubernetes allows user to do that. I think it shoud be good if we can prevent burst cpu usage from comparing all keyspaces. |
This ^, Kubernetes will not break GA feature. Implementing this on etcd is worthless until you make proposal that is accepted on Kubernetes side.
Not true, watch cache is just not used for all types of requests. KEP https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2340-Consistent-reads-from-cache is about broadening types of requests that are served from apiserver. End goal is to serve all requests from cache. |
This feature can be useful to check the health of serializable read: use the first key to check reading from bolt db is ok. Existing check uses Range(non-exist key), which does not hit the db file. More context in #16007 |
What would you like to be added?
Present Behavior
Currently ETCD iterates on all matching keys in MVCC for Range query to calculate count in RangeResponse
Although it returns limited number of revisions from key(included) to end(excluded) at the given rev but iterates on all revisions to calculate the total number of revisions.
Change Requested
New
no-count
mode inRangeRequest
structure.By default, it will be
false
to not break current ETCDRangeResponse.Count
contract and keep backward compatibility.When explicitly set in
RangeRequest
, then ETCD will respectlimit
and break without iterating over all remaining revisions and returns fakeCount
inRangeResponse
which client/consumers should ignore.This is motivated by some previous PR and discussions, in which limit enforcement has to be reverted because of breaking ETCD contract and thus breaking K8s remainingItemCount feature.
Previous PR references:
Why is this needed?
Performance Improvement for Paginated Range query.
As with
no-count
feature, ETCD does not need to count all revisions to calculateCount
forRangeResponse
.This means retrieving X objects time is not dependent on total number of matching objects in ETCD.
Pasting some results from my experiment.
Here :
Etcd InstanceType:
m5.4xlarge
Test cmd: Get first 2 game-config objects from large number of game-configs present in ETCD (Column 2 in below results)
Time only for Revisions() columns in above measures time only for this code block
Please let me know if this change request look good then I can prepare PR for code changes.
The text was updated successfully, but these errors were encountered: