Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vitessdriver|vtadmin] Support Ping in vitessdriver, use in vtadmin to healthcheck connections during Dial #7709

Merged
merged 4 commits into from
Mar 18, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Mar 17, 2021

Description

This PR fixes a longstanding issue in vtadmin, where if vtadmin had Dial-ed a cluster, obtained a connection to a vtgate, and that vtgate was torn down, that cluster would become unusable without restarting vtadmin, and we would see the following, forever:

W0317 12:23:29.496680       7 clientconn.go:1191] grpc: addrConn.createTransport failed to connect to {${somehostname} 0  <nil>}. Err :connection error: desc = "transport: Error while dialing dial tcp ${someaddr}: connect: connection refused". Reconnecting...

To fix this, I've introduced the following:

  • An implementation of the database/sq/driver.Pinger interface in vitessdriver, which just issues a select 1 query.
  • A call to Ping during VTGateProxy.Dial in the case where the proxy has a non-nil connection. We set a configurable timeout here, to avoid blocking forever, and if the call fails we attempt to close the connection and then fall through to the "discover new vtgate and dial it" phase of the Dial method. If the ping check succeeds, we know we can just keep using that connection.

Related Issue(s)

Checklist

  • Should this PR be backported? no
  • Tests were added or are not required n/a
  • Documentation was added or is not required

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving (maybe?)
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@@ -132,13 +136,27 @@ func (vtgate *VTGateProxy) Dial(ctx context.Context, target string, opts ...grpc
vtgate.annotateSpan(span)

if vtgate.conn != nil {
span.Annotate("is_noop", true)

// (TODO:@amason): consider a quick Ping() check in this case, and get a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good suggestion, turns out!

Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ajm188
Copy link
Contributor Author

ajm188 commented Mar 17, 2021

Looks like the onlineddl_devert test is having trouble downloading a dependency from mongodb:

Fetched 4582 kB in 5s (876 kB/s)
Reading package lists...
E: Failed to fetch https://repo.mongodb.org/apt/ubuntu/dists/focal/mongodb-org/4.4/multiverse/binary-arm64/Packages.gz  File has unexpected size (6835 != 6951). Mirror sync in progress? [IP: 13.32.207.84 443]
   Hashes of expected file:
    - Filesize:6951 [weak]
    - SHA512:4053c0cdc0bc113a998eacc500646779caff4d678a0739a8ff26d7c43775191062122d34220a11d9f5a5ddd6dbc407c55a1509142ee0ba10faae3a51c7017104
    - SHA256:b4ff636e06c00ebe0fca9ffa321e3574c4e4a5a075b542d7cb9690020b90a0fc
    - SHA1:22f631d805a9cc118dae094ddbd71300ce7f13b2 [weak]
    - MD5Sum:8e0a7a78501118d1e34a7bbb922d811c [weak]
   Release file created at: Wed, 17 Mar 2021 17:14:18 +0000
E: Some index files failed to download. They have been ignored, or old ones used instead.
Error: Process completed with exit code 100.

Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@deepthi deepthi merged commit 0958ad5 into vitessio:master Mar 18, 2021
@askdba askdba added the Component: VTAdmin VTadmin interface label Mar 18, 2021
@askdba askdba added this to the v10.0 milestone Mar 18, 2021
@ajm188 ajm188 added Type: Bug Type: Enhancement Logical improvement (somewhere between a bug and feature) labels May 21, 2021
@ajm188 ajm188 mentioned this pull request Mar 30, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VTAdmin VTadmin interface Type: Bug Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants