-
Notifications
You must be signed in to change notification settings - Fork 136
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
tidb_client: improve behavior when no alive tidb instance #900
Conversation
f.statusProxy.updateRemotes(nil) | ||
} | ||
} else { | ||
if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errorx.IsOfType(err, ErrNoAliveTiDB)
is a legacy code and is always false, so I simply remove entire err != nil
block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest LGTM. /cc @shhdgit PTAL
LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
@@ -111,7 +111,10 @@ func (c *Client) OpenSQLConn(user string, pass string) (*gorm.DB, error) { | |||
if overrideEndpoint != "" { | |||
addr = overrideEndpoint | |||
} else { | |||
addr = fmt.Sprintf("127.0.0.1:%d", c.forwarder.sqlPort) | |||
var err error | |||
if addr, err = c.forwarder.getEndpointAddr(c.forwarder.sqlPort); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about getSQLEndpointAddr
?
@@ -158,10 +164,19 @@ func (c *Client) SendGetRequest(path string) ([]byte, error) { | |||
if overrideEndpoint != "" { | |||
addr = overrideEndpoint | |||
} else { | |||
addr = fmt.Sprintf("127.0.0.1:%d", c.forwarder.statusPort) | |||
var err error | |||
if addr, err = c.forwarder.getEndpointAddr(c.forwarder.statusPort); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about getStatusEndpointAddr
?
@@ -118,6 +113,14 @@ func (f *Forwarder) pollingForTiDB() { | |||
} | |||
} | |||
|
|||
func (f *Forwarder) getEndpointAddr(port int) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
Conflicts: pkg/tidb/client.go
Conflicts: pkg/tidb/client.go
* fix(ttlcache): goroutine leak (#892) * tidb: forwarder only uses tidb whose status is Up (#893) * keyviz: Add tips for enabled config (#901) * ui: rocksdb fields (#896) * monitoring: setup sentry (#895) * tidb_client: improve behavior when no alive tidb instance (#900) * feat(stmt): support config maximum number of stmt kept in memory (#914) * feat: debug api (#898) * ui: Improve settings description for Statement (#920) * feat(ui): add tiflash profiling option (#859) * ui: Add a warning for the debug API (#922)
closes #899