Skip to content

[WIP] Carry on 21261#21465

Open
fuweid wants to merge 14 commits intoetcd-io:mainfrom
fuweid:carry-on-21261
Open

[WIP] Carry on 21261#21465
fuweid wants to merge 14 commits intoetcd-io:mainfrom
fuweid:carry-on-21261

Conversation

@fuweid
Copy link
Copy Markdown
Member

@fuweid fuweid commented Mar 9, 2026

No description provided.

@k8s-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fuweid

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 44.14894% with 420 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.36%. Comparing base (50f4ea2) to head (a756d13).
⚠️ Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
etcdctl/ctlv3/command/printer_fields.go 0.00% 120 Missing ⚠️
etcdctl/ctlv3/command/printer_simple.go 0.00% 86 Missing ⚠️
etcdctl/ctlv3/command/printer.go 0.00% 72 Missing ⚠️
etcdctl/ctlv3/command/printer_json.go 70.00% 18 Missing ⚠️
etcdctl/ctlv3/command/printer_protobuf.go 0.00% 13 Missing ⚠️
client/v3/compare.go 76.74% 7 Missing and 3 partials ⚠️
etcdctl/ctlv3/command/user_command.go 0.00% 8 Missing ⚠️
etcdutl/etcdutl/printer_protobuf.go 0.00% 8 Missing ⚠️
etcdctl/ctlv3/command/watch_command.go 0.00% 7 Missing ⚠️
server/storage/wal/wal.go 70.83% 4 Missing and 3 partials ⚠️
... and 34 more
Additional details and impacted files
Files with missing lines Coverage Δ
api/mvccpb/extension.go 100.00% <100.00%> (ø)
client/v3/concurrency/election.go 79.36% <100.00%> (-2.67%) ⬇️
client/v3/leasing/cache.go 87.42% <100.00%> (-3.69%) ⬇️
client/v3/leasing/kv.go 90.39% <100.00%> (-1.00%) ⬇️
client/v3/leasing/util.go 94.44% <100.00%> (-3.89%) ⬇️
client/v3/mirror/syncer.go 76.74% <100.00%> (ø)
client/v3/namespace/kv.go 73.91% <100.00%> (ø)
client/v3/op.go 77.09% <100.00%> (+1.44%) ⬆️
client/v3/txn.go 100.00% <100.00%> (ø)
etcdutl/snapshot/v3_snapshot.go 58.30% <100.00%> (ø)
... and 68 more

... and 13 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21465      +/-   ##
==========================================
- Coverage   68.47%   68.36%   -0.11%     
==========================================
  Files         428      430       +2     
  Lines       35291    35439     +148     
==========================================
+ Hits        24165    24229      +64     
- Misses       9730     9806      +76     
- Partials     1396     1404       +8     

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 50f4ea2...a756d13. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fuweid
Copy link
Copy Markdown
Member Author

fuweid commented Mar 16, 2026

/retest

fuweid and others added 4 commits March 17, 2026 15:25
Add client/v3/internal/pb/clone.go as a temporary gogo clone helper,
keep clone boundaries when txn/op code retains compare state, and update
the namespace/leasing/grpcproxy/etcdctl/robustness paths to use the new
representation.

```
goos: linux
goarch: amd64
pkg: go.etcd.io/etcd/client/v3
cpu: AMD Ryzen 7 5800H with Radeon Graphics
			     │ /tmp/bench-old.txt │           /tmp/bench-new.txt            │    /tmp/bench-new-without-clone.txt     │
			     │       sec/op       │    sec/op      vs base                  │    sec/op      vs base                  │
TxnIfSingleCmp-16                          160.6n ± ∞ ¹   1383.0n ± ∞ ¹         ~ (p=1.000 n=1) ²    643.2n ± ∞ ¹         ~ (p=1.000 n=1) ²
KubernetesOptimisticPutTxnBuild-16         953.5n ± ∞ ¹   2286.0n ± ∞ ¹         ~ (p=1.000 n=1) ²   1494.0n ± ∞ ¹         ~ (p=1.000 n=1) ²
OpTxnSingleCmpToTxnRequest-16              880.1n ± ∞ ¹   2794.0n ± ∞ ¹         ~ (p=1.000 n=1) ²   1247.0n ± ∞ ¹         ~ (p=1.000 n=1) ²
geomean                                    512.7n          2.067µ        +303.19%                    1.062µ        +107.17%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

			     │ /tmp/bench-old.txt │           /tmp/bench-new.txt           │   /tmp/bench-new-without-clone.txt    │
			     │        B/op        │     B/op       vs base                 │     B/op       vs base                │
TxnIfSingleCmp-16                           304.0 ± ∞ ¹     600.0 ± ∞ ¹        ~ (p=1.000 n=1) ²     448.0 ± ∞ ¹       ~ (p=1.000 n=1) ²
KubernetesOptimisticPutTxnBuild-16        2.008Ki ± ∞ ¹   2.297Ki ± ∞ ¹        ~ (p=1.000 n=1) ²   2.148Ki ± ∞ ¹       ~ (p=1.000 n=1) ²
OpTxnSingleCmpToTxnRequest-16             1.867Ki ± ∞ ¹   1.844Ki ± ∞ ¹        ~ (p=1.000 n=1) ²   1.539Ki ± ∞ ¹       ~ (p=1.000 n=1) ²
geomean                                   1.036Ki         1.354Ki        +30.64%                   1.131Ki        +9.13%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

			     │ /tmp/bench-old.txt │          /tmp/bench-new.txt           │   /tmp/bench-new-without-clone.txt   │
			     │     allocs/op      │  allocs/op    vs base                 │  allocs/op   vs base                 │
TxnIfSingleCmp-16                           5.000 ± ∞ ¹   10.000 ± ∞ ¹        ~ (p=1.000 n=1) ²   7.000 ± ∞ ¹        ~ (p=1.000 n=1) ²
KubernetesOptimisticPutTxnBuild-16          22.00 ± ∞ ¹    27.00 ± ∞ ¹        ~ (p=1.000 n=1) ²   24.00 ± ∞ ¹        ~ (p=1.000 n=1) ²
OpTxnSingleCmpToTxnRequest-16               21.00 ± ∞ ¹    28.00 ± ∞ ¹        ~ (p=1.000 n=1) ²   21.00 ± ∞ ¹        ~ (p=1.000 n=1) ³
geomean                                     13.22          19.63        +48.47%                   15.22        +15.16%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05
³ all samples are equal
```

Co-authored-by: Jordan Liggitt <liggitt@google.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Switch concurrency.Election.Observe and mirror.Syncer.SyncBase
from GetResponse values to *GetResponse.

Update the etcdctl election observer and add integration coverage for
multiple Observe updates to verify each event returns a fresh
response wrapper.

Also manually copy lease cache responses/headers to avoid shared mutable
state in future.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Keep close/progress responses safe by always providing a non-nil header,
and update downstream users to match pointer semantics:

- cache demux progress responses
- concurrency election observe path
- grpcproxy watcher/watch cancel path
- etcdctl watch printers

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
switch ctlv3 command printer/watch paths to pointer-based gogopb response
handling, and read protobuf-backed fields via GetXXX() just in case input
is nil.

In tests/framework/e2e/etcdctl.go, txn output is decoded into
command.TxnResponseJSON and converted with ToProto(), instead of hand-building
ResponseOp variants before json.Unmarshal.

Co-authored-by: Jordan Liggitt <liggitt@google.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added 8 commits March 17, 2026 21:38
keep WAL snapshot tracking on pointer form as well:
- change WAL.start from walpb.Snapshot to *walpb.Snapshot
- in openAtIndex, normalize nil input to &walpb.Snapshot{} and store pointer start
- keep ReadAll math/error path on getters (GetIndex/GetTerm) for pointer-safe access
- update WAL test fixture to pass start as pointer

Signed-off-by: Wei Fu <fuweid89@gmail.com>
stop passing mvcc gogopb structs around by value in txn/watch paths.

- change mvcc interfaces/results to pointer forms:
  - RangeResult.KVs: []mvccpb.KeyValue -> []*mvccpb.KeyValue
  - TxnWrite.Changes: []mvccpb.KeyValue -> []*mvccpb.KeyValue
  - watch event batches: []mvccpb.Event -> []*mvccpb.Event
- update txn rpc assembly to reuse pointers directly instead of &slice[i] copies
- update watch send/filter pipeline (mvcc, v3rpc, grpcproxy) to pointer events
- update sendFragments to build per-fragment WatchResponse explicitly (avoid proto struct value copy)
  - add TestWatchResponseProtoFieldCount to catch future WatchResponse field additions
- adjust compare/filter helpers to pointer args
- update affected mvcc/watch tests accordingly

Signed-off-by: Wei Fu <fuweid89@gmail.com>
switch revKeyValue.kv from mvccpb.KeyValue to *mvccpb.KeyValue and
unmarshal into a newly allocated KeyValue in restoreChunk.

this removes gogopb-by-value usage in restore/index rebuild and keeps
pb handling aligned with pointer-based paths.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
…t path

Switch EtcdServer.raftRequestOnce, EtcdServer.raftRequest, and
EtcdServer.processInternalRaftRequestOnce to take *etcdserverpb.InternalRaftRequest.

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Also using String() instead of %+v for proto struct

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Wei Fu <fuweid89@gmail.com>
@fuweid
Copy link
Copy Markdown
Member Author

fuweid commented Mar 18, 2026

/retest

@k8s-ci-robot
Copy link
Copy Markdown

PR needs rebase.

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.

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

Development

Successfully merging this pull request may close these issues.

2 participants