client: supports to add gRPC dial options #2035

nolouch · 2019-12-18T07:45:00Z

Signed-off-by: nolouch nolouch@gmail.com

What problem does this PR solve?

we test
After all 3 instances of PD are killed in AWS(k8s environment), it takes a long time (15 minutes) for TiDB server instances to reconnect to new PD instances. and we found the stale TCP connection after all pod IP is changed.

 we see the connection is still establish, and that ip does not exist after kill.
10.0.48.116 is not exist, but the tcp is establisted.

Tue Dec 17 12:54:36 UTC 2019
tcp        0      1 10.0.38.181:53424       172.20.62.78:2379       SYN_SENT    1/tidb-server
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0   1140 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server
Tue Dec 17 12:54:37 UTC 2019
tcp        0      1 10.0.38.181:53424       172.20.62.78:2379       SYN_SENT    1/tidb-server
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0   1153 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server
Tue Dec 17 12:54:37 UTC 2019
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53488       172.20.62.78:2379       ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0   1153 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server

....

Tue Dec 17 13:07:15 UTC 2019
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53488       172.20.62.78:2379       ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0  26868 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server
Tue Dec 17 13:07:16 UTC 2019
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53488       172.20.62.78:2379       ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0  26868 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server
Tue Dec 17 13:07:16 UTC 2019
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53488       172.20.62.78:2379       ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0  26868 10.0.38.181:35378       10.0.48.116:2379        ESTABLISHED 1/tidb-server
Tue Dec 17 13:07:17 UTC 2019
tcp        0      0 10.0.38.181:53302       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53304       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53488       172.20.62.78:2379       ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:53300       10.0.22.167:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:37554       10.0.46.225:2379        ESTABLISHED 1/tidb-server
tcp        0      0 10.0.38.181:41754       10.0.55.94:2379         ESTABLISHED 1/tidb-server

This problem same as pingcap/tidb#7099. may k8s CNI dropping all packets send to the removed node(Indeterminate), that cause a stall conneciton, until kernel TCP retransmission times out and closes the connection.

What is changed and how it works?

supports add gRPC dial options and setting gRPC KeepAlive in tidb's pd client, this problem fixed.

Check List

Tests

Unit test

Signed-off-by: nolouch <nolouch@gmail.com>

codecov-io · 2019-12-18T08:17:30Z

Codecov Report

Merging #2035 into master will decrease coverage by 0.13%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master    #2035      +/-   ##
==========================================
- Coverage   76.94%   76.81%   -0.14%     
==========================================
  Files         183      183              
  Lines       18335    18335              
==========================================
- Hits        14108    14084      -24     
- Misses       3163     3181      +18     
- Partials     1064     1070       +6

Impacted Files	Coverage Δ
server/region_syncer/client.go	`82.27% <77.77%> (+2.53%)`	⬆️
server/schedulers/shuffle_hot_region.go	`62.62% <0%> (-5.06%)`	⬇️
server/member/leader.go	`76.53% <0%> (-4.6%)`	⬇️
server/kv/etcd_kv.go	`81.81% <0%> (-2.6%)`	⬇️
client/client.go	`72.23% <0%> (-1.82%)`	⬇️
server/core/storage.go	`74.25% <0%> (-1.2%)`	⬇️
server/config/option.go	`90.32% <0%> (-0.93%)`	⬇️
pkg/btree/btree.go	`86.84% <0%> (-0.81%)`	⬇️
server/handler.go	`48.91% <0%> (-0.44%)`	⬇️
server/server.go	`82.65% <0%> (-0.39%)`	⬇️
... and 4 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 2074dbd...5c609c1. Read the comment docs.

Signed-off-by: nolouch <nolouch@gmail.com>

client/client.go

rleungx

LGTM

lhy1024

the rest LGTM

client/client.go

bb7133 · 2019-12-18T11:04:49Z

client/client.go

+// ClientOption configures client.
+type ClientOption func(c *client)
+
+// WithGRPCDialOptions configures the client with gRPC dial options.


For now, we have no gRPC related parameter exposed in Client, to keep the interface implementation-independent, how about defining a new Option type and map it to grpc.DialOption?

That's means if I want to add a new gRPC option, I need to map it again and then update pd in tidb again.

you can check etcd client also exposed gRPC option:

https://github.com/etcd-io/etcd/blob/5770a6d286fe48682e29b54ce0df37e7d24b3280/clientv3/config.go#L70-L73

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch · 2019-12-19T04:14:26Z

PTAL @disksing @lhy1024

nolouch · 2019-12-19T05:34:50Z

/merge

sre-bot · 2019-12-19T05:34:54Z

Your auto merge job has been accepted, waiting for 2040

sre-bot · 2019-12-19T05:39:17Z

/run-all-tests

sre-bot · 2019-12-19T05:46:05Z

cherry pick to release-3.0 failed

sre-bot · 2019-12-19T05:46:19Z

cherry pick to release-3.1 failed

Signed-off-by: nolouch <nolouch@gmail.com>

client: supports to add gRPC dial options

360357e

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch requested review from disksing and rleungx December 18, 2019 07:45

nolouch added the component/client Client logic. label Dec 18, 2019

nolouch requested a review from shafreeck December 18, 2019 08:08

fix ci

b1318f8

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the add-grpc-option branch from 09ad54d to b1318f8 Compare December 18, 2019 08:19

nolouch added 2 commits December 18, 2019 00:19

Merge branch 'master' into add-grpc-option

d932988

add break

b2b25dc

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch mentioned this pull request Dec 18, 2019

store/tikv: keepalive with pd pingcap/tidb#14118

Merged

disksing reviewed Dec 18, 2019

View reviewed changes

client/client.go Outdated Show resolved Hide resolved

rleungx approved these changes Dec 18, 2019

View reviewed changes

lhy1024 reviewed Dec 18, 2019

View reviewed changes

client/client.go Outdated Show resolved Hide resolved

bb7133 reviewed Dec 18, 2019

View reviewed changes

nolouch added 2 commits December 19, 2019 11:41

address comments

419ca76

Signed-off-by: nolouch <nolouch@gmail.com>

rename

2074dbd

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch added needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. needs-cherry-pick-release-3.1 The PR needs to cherry pick to release-3.1 branch. labels Dec 19, 2019

nolouch added this to the v4.0.0-beta milestone Dec 19, 2019

shafreeck approved these changes Dec 19, 2019

View reviewed changes

lhy1024 approved these changes Dec 19, 2019

View reviewed changes

sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 19, 2019

Merge branch 'master' into add-grpc-option

5c609c1

sre-bot merged commit 4d65bbe into tikv:master Dec 19, 2019

nolouch added a commit to nolouch/pd that referenced this pull request Dec 23, 2019

client: supports to add gRPC dial options (tikv#2035)

43162f9

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch mentioned this pull request Dec 23, 2019

client: supports to add gRPC dial options (#2035) #2043

Merged

sre-bot pushed a commit that referenced this pull request Dec 23, 2019

client: supports to add gRPC dial options (#2035) (#2043)

ea2b748

Signed-off-by: nolouch <nolouch@gmail.com>

sre-bot pushed a commit to sre-bot/pd that referenced this pull request Dec 23, 2019

client: supports to add gRPC dial options (tikv#2035)

6ac15df

Signed-off-by: nolouch <nolouch@gmail.com>

sre-bot mentioned this pull request Dec 23, 2019

client: supports to add gRPC dial options (#2035) (#2043) #2046

Merged

sre-bot added a commit that referenced this pull request Dec 24, 2019

client: supports to add gRPC dial options (#2035) (#2043) (#2046)

529de48

Signed-off-by: nolouch <nolouch@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: supports to add gRPC dial options #2035

client: supports to add gRPC dial options #2035

nolouch commented Dec 18, 2019 •

edited

Loading

codecov-io commented Dec 18, 2019 •

edited

Loading

rleungx left a comment

lhy1024 left a comment

bb7133 Dec 18, 2019

nolouch Dec 18, 2019

nolouch Dec 18, 2019

nolouch commented Dec 19, 2019

nolouch commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

client: supports to add gRPC dial options #2035

client: supports to add gRPC dial options #2035

Conversation

nolouch commented Dec 18, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov-io commented Dec 18, 2019 • edited Loading

Codecov Report

rleungx left a comment

Choose a reason for hiding this comment

lhy1024 left a comment

Choose a reason for hiding this comment

bb7133 Dec 18, 2019

Choose a reason for hiding this comment

nolouch Dec 18, 2019

Choose a reason for hiding this comment

nolouch Dec 18, 2019

Choose a reason for hiding this comment

nolouch commented Dec 19, 2019

nolouch commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

sre-bot commented Dec 19, 2019

nolouch commented Dec 18, 2019 •

edited

Loading

codecov-io commented Dec 18, 2019 •

edited

Loading