clientv3: fix "hasleader" metadata key, embed client version #11687

gyuho · 2020-03-09T22:05:37Z

WithRequireLeader was overwriting existing metadata key-values
Add client-api-version metadata (useful for implementing client-version based server code)

e.g. Example metadata output in server side:

map[:authority:[localhost:64304770101484282250] client-api-version:[3.5] content-type:[application/grpc] user-agent:[grpc-go/1.26.0]]

gyuho · 2020-03-09T22:05:49Z

/cc @jpbetz @xiang90 @jingyih

codecov-io · 2020-03-09T22:45:15Z

Codecov Report

Merging #11687 into master will decrease coverage by 0.2%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #11687      +/-   ##
==========================================
- Coverage    66.4%   66.19%   -0.21%     
==========================================
  Files         402      403       +1     
  Lines       36667    36697      +30     
==========================================
- Hits        24349    24292      -57     
- Misses      10829    10901      +72     
- Partials     1489     1504      +15

Impacted Files	Coverage Δ
clientv3/client.go	`72.3% <ø> (-2.56%)`	⬇️
clientv3/ctx.go	`100% <100%> (ø)`
etcdserver/api/v3rpc/interceptor.go	`46% <100%> (+3.04%)`	⬆️
etcdserver/api/v3rpc/metrics.go	`100% <100%> (ø)`	⬆️
clientv3/retry_interceptor.go	`68.62% <100%> (-0.68%)`	⬇️
integration/cluster.go	`85.58% <100%> (+0.15%)`	⬆️
auth/options.go	`47.5% <0%> (-45%)`	⬇️
pkg/transport/timeout_conn.go	`60% <0%> (-20%)`	⬇️
etcdserver/api/v3rpc/util.go	`51.61% <0%> (-16.13%)`	⬇️
client/client.go	`55.55% <0%> (-15.04%)`	⬇️
... and 26 more

Continue to review full report at Codecov.

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

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho · 2020-03-17T19:17:10Z

@jingyih @wenjiaswe Please take a look.This also includes a bug fix for "hasleader" metadata key.

jingyih

LGTM. Thanks! This is useful!

jingyih · 2020-03-18T14:09:21Z

Is overwriting metadata related.to any known / reported bug?

gyuho · 2020-03-18T16:17:04Z

Is overwriting metadata related.to any known / reported bug?

Not that I know of. Just found it when I was adding client version metadata :)

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com> clientv3: fix racy writes to context key === RUN TestWatchOverlapContextCancel ================== WARNING: DATA RACE Write at 0x00c42110dd40 by goroutine 99: runtime.mapassign() /usr/local/go/src/runtime/hashmap.go:485 +0x0 github.com/coreos/etcd/clientv3.metadataSet() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:61 +0x8c github.com/coreos/etcd/clientv3.withVersion() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:47 +0x137 github.com/coreos/etcd/clientv3.newStreamClientInterceptor.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/client.go:309 +0x81 google.golang.org/grpc.NewClientStream() /go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/stream.go:101 +0x10e github.com/coreos/etcd/etcdserver/etcdserverpb.(*watchClient).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3193 +0xe9 github.com/coreos/etcd/clientv3.(*watchGrpcStream).openWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:788 +0x143 github.com/coreos/etcd/clientv3.(*watchGrpcStream).newWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:700 +0x5c3 github.com/coreos/etcd/clientv3.(*watchGrpcStream).run() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:431 +0x12b Previous read at 0x00c42110dd40 by goroutine 130: reflect.maplen() /usr/local/go/src/runtime/hashmap.go:1165 +0x0 reflect.Value.MapKeys() /usr/local/go/src/reflect/value.go:1090 +0x43b fmt.(*pp).printValue() /usr/local/go/src/fmt/print.go:741 +0x1885 fmt.(*pp).printArg() /usr/local/go/src/fmt/print.go:682 +0x1b1 fmt.(*pp).doPrintf() /usr/local/go/src/fmt/print.go:998 +0x1cad fmt.Sprintf() /usr/local/go/src/fmt/print.go:196 +0x77 github.com/coreos/etcd/clientv3.streamKeyFromCtx() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:825 +0xc8 github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:265 +0x426 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 99 (running) created at: github.com/coreos/etcd/clientv3.(*watcher).newWatcherGrpcStream() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:236 +0x59d github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:278 +0xbb6 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 130 (running) created at: github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:979 +0x76d github.com/coreos/etcd/clientv3/integration.TestWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:922 +0x44 testing.tRunner() /usr/local/go/src/testing/testing.go:657 +0x107 ================== Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com> clientv3: fix racy writes to context key === RUN TestWatchOverlapContextCancel ================== WARNING: DATA RACE Write at 0x00c42110dd40 by goroutine 99: runtime.mapassign() /usr/local/go/src/runtime/hashmap.go:485 +0x0 github.com/coreos/etcd/clientv3.metadataSet() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:61 +0x8c github.com/coreos/etcd/clientv3.withVersion() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:47 +0x137 github.com/coreos/etcd/clientv3.newStreamClientInterceptor.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/client.go:309 +0x81 google.golang.org/grpc.NewClientStream() /go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/stream.go:101 +0x10e github.com/coreos/etcd/etcdserver/etcdserverpb.(*watchClient).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3193 +0xe9 github.com/coreos/etcd/clientv3.(*watchGrpcStream).openWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:788 +0x143 github.com/coreos/etcd/clientv3.(*watchGrpcStream).newWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:700 +0x5c3 github.com/coreos/etcd/clientv3.(*watchGrpcStream).run() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:431 +0x12b Previous read at 0x00c42110dd40 by goroutine 130: reflect.maplen() /usr/local/go/src/runtime/hashmap.go:1165 +0x0 reflect.Value.MapKeys() /usr/local/go/src/reflect/value.go:1090 +0x43b fmt.(*pp).printValue() /usr/local/go/src/fmt/print.go:741 +0x1885 fmt.(*pp).printArg() /usr/local/go/src/fmt/print.go:682 +0x1b1 fmt.(*pp).doPrintf() /usr/local/go/src/fmt/print.go:998 +0x1cad fmt.Sprintf() /usr/local/go/src/fmt/print.go:196 +0x77 github.com/coreos/etcd/clientv3.streamKeyFromCtx() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:825 +0xc8 github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:265 +0x426 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 99 (running) created at: github.com/coreos/etcd/clientv3.(*watcher).newWatcherGrpcStream() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:236 +0x59d github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:278 +0xbb6 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 130 (running) created at: github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:979 +0x76d github.com/coreos/etcd/clientv3/integration.TestWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:922 +0x44 testing.tRunner() /usr/local/go/src/testing/testing.go:657 +0x107 ================== Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com> clientv3: fix racy writes to context key === RUN TestWatchOverlapContextCancel ================== WARNING: DATA RACE Write at 0x00c42110dd40 by goroutine 99: runtime.mapassign() /usr/local/go/src/runtime/hashmap.go:485 +0x0 github.com/coreos/etcd/clientv3.metadataSet() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:61 +0x8c github.com/coreos/etcd/clientv3.withVersion() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/ctx.go:47 +0x137 github.com/coreos/etcd/clientv3.newStreamClientInterceptor.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/client.go:309 +0x81 google.golang.org/grpc.NewClientStream() /go/src/github.com/coreos/etcd/gopath/src/google.golang.org/grpc/stream.go:101 +0x10e github.com/coreos/etcd/etcdserver/etcdserverpb.(*watchClient).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3193 +0xe9 github.com/coreos/etcd/clientv3.(*watchGrpcStream).openWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:788 +0x143 github.com/coreos/etcd/clientv3.(*watchGrpcStream).newWatchClient() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:700 +0x5c3 github.com/coreos/etcd/clientv3.(*watchGrpcStream).run() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:431 +0x12b Previous read at 0x00c42110dd40 by goroutine 130: reflect.maplen() /usr/local/go/src/runtime/hashmap.go:1165 +0x0 reflect.Value.MapKeys() /usr/local/go/src/reflect/value.go:1090 +0x43b fmt.(*pp).printValue() /usr/local/go/src/fmt/print.go:741 +0x1885 fmt.(*pp).printArg() /usr/local/go/src/fmt/print.go:682 +0x1b1 fmt.(*pp).doPrintf() /usr/local/go/src/fmt/print.go:998 +0x1cad fmt.Sprintf() /usr/local/go/src/fmt/print.go:196 +0x77 github.com/coreos/etcd/clientv3.streamKeyFromCtx() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:825 +0xc8 github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:265 +0x426 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 99 (running) created at: github.com/coreos/etcd/clientv3.(*watcher).newWatcherGrpcStream() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:236 +0x59d github.com/coreos/etcd/clientv3.(*watcher).Watch() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/watch.go:278 +0xbb6 github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel.func1() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:959 +0x23e Goroutine 130 (running) created at: github.com/coreos/etcd/clientv3/integration.testWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:979 +0x76d github.com/coreos/etcd/clientv3/integration.TestWatchOverlapContextCancel() /go/src/github.com/coreos/etcd/gopath/src/github.com/coreos/etcd/clientv3/integration/watch_test.go:922 +0x44 testing.tRunner() /usr/local/go/src/testing/testing.go:657 +0x107 ================== Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho force-pushed the embed-client-version branch 5 times, most recently from 165246e to c83c645 Compare March 10, 2020 06:15

gyuho added 3 commits March 17, 2020 12:16

clientv3: fix metadata overwrites in "WithRequireLeader"

5cb1e0b

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

clientv3: embed API version

58ba322

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

*: add "etcd_server_client_requests_total", tests

3390747

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho force-pushed the embed-client-version branch from c83c645 to 3390747 Compare March 17, 2020 19:16

jingyih reviewed Mar 18, 2020

View reviewed changes

gyuho merged commit 1c16c24 into etcd-io:master Mar 18, 2020

gyuho deleted the embed-client-version branch March 18, 2020 16:16

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

89a189a

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

98dddaa

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

1c6e5b6

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

412ec15

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

605ab68

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

fcc1f79

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

37c1c34

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

2b0ae8c

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

833a452

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

2e35390

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

clientv3: embed api version in metadata

de780f2

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

6c8031c

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 18, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

5afcd39

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 19, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

e63db56

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 19, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

d9027ce

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

gyuho added a commit that referenced this pull request Mar 19, 2020

etcdserver/api/v3rpc: handle api version metadata, add metrics

35d1d2d

ref. #11687 Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

hexfusion mentioned this pull request Mar 20, 2020

[openshift-4.4] Bug 1815637: bump etcd v3.3.19 openshift/etcd#40

Closed

gyuho mentioned this pull request Mar 30, 2020

Clarification on watch when server (or store) is partitioned kubernetes/kubernetes#89488

Closed

jingyih mentioned this pull request Apr 2, 2020

Update to etcd 3.4.7 in Kubernetes 1.19 release kubernetes/kubernetes#89779

Closed

2 tasks

hexfusion mentioned this pull request Apr 17, 2020

Bug 1815637: bump etcd v3.3.20 openshift/etcd#45

Closed

hexfusion mentioned this pull request May 21, 2020

Bug 1815637: bump etcd v3.3.22 openshift/etcd#47

Merged

hexfusion mentioned this pull request Jul 15, 2020

Bug 1815634: bump etcd v3.3.22 openshift/etcd#53

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clientv3: fix "hasleader" metadata key, embed client version #11687

clientv3: fix "hasleader" metadata key, embed client version #11687

gyuho commented Mar 9, 2020

gyuho commented Mar 9, 2020

codecov-io commented Mar 9, 2020 •

edited

Loading

gyuho commented Mar 17, 2020

jingyih left a comment

jingyih commented Mar 18, 2020

gyuho commented Mar 18, 2020

clientv3: fix "hasleader" metadata key, embed client version #11687

clientv3: fix "hasleader" metadata key, embed client version #11687

Conversation

gyuho commented Mar 9, 2020

gyuho commented Mar 9, 2020

codecov-io commented Mar 9, 2020 • edited Loading

Codecov Report

gyuho commented Mar 17, 2020

jingyih left a comment

Choose a reason for hiding this comment

jingyih commented Mar 18, 2020

gyuho commented Mar 18, 2020

codecov-io commented Mar 9, 2020 •

edited

Loading