etcdserver: optimize range request with KEY ASCEND sorting

[kairosci]

This PR optimizes range request handling by passing the limit to the backend when sorting by key in ascending order, as the backend already returns keys in this order. This avoids fetching all keys and then truncating in memory.

goos: windows
goarch: amd64
pkg: go.etcd.io/etcd/server/v3/etcdserver/txn
cpu: Intel(R) Core(TM) 5 120U

                                            │        old.txt        │              new.txt               │
                                            │         sec/op        │   sec/op      vs base              │
--------------------------------------------------------------------------------------------------------
RangeLimitOptimization/KeyAscendLimit100_10k/Old       3.056m ± 0%       26.93μ ± 0%   -99.12% (p=0.000)
RangeLimitOptimization/KeyAscendLimit100_100k/Old     16.639m ± 0%       27.11μ ± 0%   -99.84% (p=0.000)
RangeLimitOptimization/KeyAscendLimit100_1m/Old     168.512m ± 0%       25.22μ ± 0%   -99.99% (p=0.000)
RangeLimitOptimization/KeyAscendLimit1000_10k/Old     2.412m ± 0%      256.44μ ± 0%   -89.37% (p=0.000)
geomean                                        14.274m              49.52μ         -99.65%

Closes #20745

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kairosci
Once this PR has been reviewed and has the lgtm label, please assign siyuanfoundation for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @kairosci. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kairosci]

Addressed the review feedback:

Replaced the TestRangeLimit unit test with integration tests that test the full Range function:

• TestRangeWithSortAndLimit: Tests correctness with various sort orders, targets, and limits
• TestRangeKeyAscendOptimizationCorrectness: Validates the optimized path produces identical results to non-optimized paths
• BenchmarkRange: Benchmarks demonstrating ~40x performance improvement with KEY ASCEND sorting

The tests no longer depend on the internal rangeLimit function.

[kairosci]

@serathius could you take another look? Thank you!

[serathius]

/ok-to-test

Nont can you do a first round of review?
/cc @nwnt

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.27%. Comparing base (23085b4) to head (6ef7e66).
⚠️ Report is 203 commits behind head on main.

⚠️ Current head 6ef7e66 differs from pull request most recent head 871d697

Please upload reports for the commit 871d697 to get more accurate results.

Additional details and impacted files

Files with missing lines	Coverage Δ
server/etcdserver/txn/range.go	91.30% <100.00%> (-0.23%)	⬇️

... and 75 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21326      +/-   ##
==========================================
+ Coverage   65.55%   68.27%   +2.72%     
==========================================
  Files         428      429       +1     
  Lines       35262    35390     +128     
==========================================
+ Hits        23115    24164    +1049     
+ Misses      10737     9811     -926     
- Partials     1410     1415       +5     

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23085b4...871d697. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[serathius]

Please fix

../server/etcdserver/txn/range_test.go:194:4: formatter: use assert.Equalf (testifylint)
			assert.Equal(t, tt.wantKeys, gotKeys, "returned keys mismatch")
			^
../server/etcdserver/txn/range_test.go:195:4: formatter: use assert.Equalf (testifylint)
			assert.Equal(t, tt.wantMore, resp.More, "More flag mismatch")
			^
../server/etcdserver/txn/range_test.go:196:4: formatter: use assert.Equalf (testifylint)
			assert.Equal(t, tt.wantCount, resp.Count, "Count mismatch")
			^
../server/etcdserver/txn/range_test.go:251:4: len: use assert.Len (testifylint)
			assert.Equal(t, len(respNoSort.Kvs), len(respAscend.Kvs), "number of keys mismatch")
			^
../server/etcdserver/txn/range_test.go:253:5: formatter: use assert.Equalf (testifylint)
				assert.Equal(t, string(respNoSort.Kvs[i].Key), string(respAscend.Kvs[i].Key),
				^
../server/etcdserver/txn/range_test.go:256:4: formatter: use assert.Equalf (testifylint)
			assert.Equal(t, respNoSort.More, respAscend.More, "More flag mismatch")
			^
../server/etcdserver/txn/range_test.go:257:4: formatter: use assert.Equalf (testifylint)
			assert.Equal(t, respNoSort.Count, respAscend.Count, "Count mismatch")
			^
7 issues:

[serathius]

Please squash commits

[kairosci]

/test pull-etcd-robustness-amd64

[kairosci]

@serathius So I've rebased and removed the tests, leaving only server/etcdserver/txn/range.go in this PR. Let me know if there are any other changes to make; it should be fine.

[serathius]

Please utilize the added range benchmark to show improvement that this PR brings. See https://www.youtube.com/watch?v=jiXnzkAzy30 for how to benchmark code.

[serathius]

I analyzed the benchmarks using Daniel Martí’s approach.

Can you just paste the results like in #21486

[k8s-ci-robot]

@kairosci: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-verify	6ef7e66	link	true	/test pull-etcd-verify
pull-etcd-unit-test-386	6ef7e66	link	true	/test pull-etcd-unit-test-386
pull-etcd-unit-test-amd64	6ef7e66	link	true	/test pull-etcd-unit-test-amd64
pull-etcd-unit-test-arm64	6ef7e66	link	true	/test pull-etcd-unit-test-arm64
pull-etcd-integration-1-cpu-amd64	6ef7e66	link	true	/test pull-etcd-integration-1-cpu-amd64
pull-etcd-coverage-report	6ef7e66	link	true	/test pull-etcd-coverage-report

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.