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

Add retry for dialPumpClient #4879

Merged
merged 6 commits into from Oct 24, 2017
Merged

Add retry for dialPumpClient #4879

merged 6 commits into from Oct 24, 2017

Conversation

XuHuaiyu
Copy link
Contributor

fix #4848
PTAL @coocood

tidb.go Outdated
@@ -232,7 +231,8 @@ func RegisterLocalStore(name string, driver engine.Driver) error {
//
// The engine should be registered before creating storage.
func NewStore(path string) (kv.Storage, error) {
return newStoreWithRetry(path, defaultMaxRetries)
return newStoreWithRetry(path, 3)
//return newStoreWithRetry(path, util.DefaultMaxRetries)
Copy link
Member

Choose a reason for hiding this comment

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

wrong commit here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@XuHuaiyu XuHuaiyu changed the title 1. Add retry for dialPumpClient 2. make start TiDB server and connecting to binlog and pd simultaneous Add retry for dialPumpClient Oct 24, 2017
tidb_test.go Outdated
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "must fail")
elapse := time.Since(begin)
c.Assert(uint64(elapse), GreaterEqual, uint64(3*time.Second))
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a smaller wait time for the test?

@coocood
Copy link
Member

coocood commented Oct 24, 2017

Please fix CI and conflict

@XuHuaiyu
Copy link
Contributor Author

PTAL @coocood

@coocood
Copy link
Member

coocood commented Oct 24, 2017

LGTM

@XuHuaiyu XuHuaiyu added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 24, 2017
@XuHuaiyu
Copy link
Contributor Author

PTAL @shenli

tidb_test.go Outdated
func (s *testMainSuite) TestRetryDialPumpClient(c *C) {
retryDialPumlClientMustFail := func(binlogSocket string, clientCon *grpc.ClientConn, maxRetries int, dialerOpt grpc.DialOption) (err error) {
return util.RunWithRetry(maxRetries, 10, func() (bool, error) {
clientCon, err = grpc.Dial(binlogSocket, grpc.WithInsecure(), dialerOpt)
Copy link
Contributor

Choose a reason for hiding this comment

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

This code doesn't use. We can remove it or add some comments.

@@ -248,13 +247,30 @@ func newStoreWithRetry(path string, maxRetries int) (kv.Storage, error) {
}

var s kv.Storage
err1 := util.RunWithRetry(maxRetries, retryInterval, func() (bool, error) {
err1 := util.RunWithRetry(maxRetries, util.RetryInterval, func() (bool, error) {
log.Infof("new store")
Copy link
Member

Choose a reason for hiding this comment

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

Why we log this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For recording this process.

tidb.go Outdated
return util.RunWithRetry(maxRetries, util.RetryInterval, func() (bool, error) {
log.Infof("setup binlog client")
var err error
clientCon, err = grpc.Dial(binlogSocket, grpc.WithInsecure(), dialerOpt)
Copy link
Member

Choose a reason for hiding this comment

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

We should log something if meet this error.

@XuHuaiyu
Copy link
Contributor Author

PTAL @shenli @zimulala

@shenli
Copy link
Member

shenli commented Oct 24, 2017

LGTM

@shenli
Copy link
Member

shenli commented Oct 24, 2017

/run-all-tests

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 24, 2017
@shenli shenli merged commit 8c7a090 into pingcap:master Oct 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check pd or pump status at bootstrap stage
5 participants