Fix https://github.com/etcd-io/etcd/issues/12385

It's still a WIP PR, please do not merge. Remaining todos:

• [ ] clean up code
• [ ] write a test

Codecov Report

Merging #14322 (c561452) into main (b886bbc) will decrease coverage by 0.18%. The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main   #14322      +/-   ##
==========================================
- Coverage   75.39%   75.21%   -0.19%     
==========================================
  Files         457      457              
  Lines       37207    37235      +28     
==========================================
- Hits        28053    28005      -48     
- Misses       7394     7455      +61     
- Partials     1760     1775      +15     

Flag	Coverage Δ
all	75.21% <72.72%> (-0.19%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
client/v3/watch.go	92.59% <66.66%> (-1.19%)	:arrow_down:
server/etcdserver/api/v3rpc/watch.go	84.29% <77.77%> (-2.00%)	:arrow_down:
client/v3/leasing/util.go	91.66% <0.00%> (-6.67%)	:arrow_down:
client/v3/namespace/watch.go	87.87% <0.00%> (-6.07%)	:arrow_down:
raft/rafttest/node.go	95.00% <0.00%> (-5.00%)	:arrow_down:
client/v3/concurrency/session.go	88.63% <0.00%> (-4.55%)	:arrow_down:
server/etcdserver/api/v3rpc/util.go	70.96% <0.00%> (-3.23%)	:arrow_down:
server/etcdserver/cluster_util.go	70.35% <0.00%> (-3.17%)	:arrow_down:
server/proxy/grpcproxy/watch.go	93.64% <0.00%> (-2.90%)	:arrow_down:
pkg/traceutil/trace.go	96.15% <0.00%> (-1.93%)	:arrow_down:
... and 16 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Thanks @mitake , just marked this PR as draft. Please feel free to mark it as ready for review once it's ready.

I clone locally, tried a run and the problem was solved, the previous watch will also be send by nextResume.

However, because the default WatchID = 0 is returned by error, this will cause WatchID = 0 to be deleted once in ids, although it will be recreated later. See https://github.com/etcd-io/etcd/pull/14296.

I also read the committed code, instead of using a string, would try passing an error code between the client and the server? And why not

err := sws.isWatchPermitted(creq)
if err != nil {

Thanks for trying this PR @kafuu-chino .

I also read the committed code, instead of using a string, would try passing an error code between the client and the server?

It might be simpler but the error codes don't have unique integer IDs. So keeping the type of cancelReason as string is good.

I'll also change the return value type of isWatchPermitted().

@ahrtr @kafuu-chino I think this PR is ready to be reviewed, could you check when you have time?

The pipeline failure should be caused by this PR. Please fix the failures.

@ahrtr @kafuu-chino Thanks for reviewing! The failed check caused by the previous version of the PR was e2e IIRC, but I couldn't reproduce the failure on my local env, I guess it was a non deterministic issue. Let me merge this branch.

I'll open PRs for backporting the change to stable releases later.