From 5c4c8204aeb8eca110439574ea63596c6901752b Mon Sep 17 00:00:00 2001
From: csuzhangxc <csuzhangxc@gmail.com>
Date: Thu, 21 Nov 2019 16:29:11 +0800
Subject: [PATCH] *: address comment

---
 pkg/election/election.go      | 24 +++++++++++++-----------
 pkg/election/election_test.go |  9 +++++----
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/pkg/election/election.go b/pkg/election/election.go
index c6b9ddeae1..fdc2fbbef1 100644
--- a/pkg/election/election.go
+++ b/pkg/election/election.go
@@ -37,8 +37,6 @@ const (
 	newSessionRetryUnlimited = math.MaxInt64
 	// newSessionRetryInterval is the interval time when retrying to create a new session.
 	newSessionRetryInterval = 200 * time.Millisecond
-	// logNewSessionIntervalCnt is the interval count when logging errors for creating session
-	logNewSessionIntervalCnt = int(3 * time.Second / newSessionRetryInterval)
 )
 
 // Election implements the leader election based on etcd.
@@ -151,7 +149,6 @@ func (e *Election) campaignLoop(ctx context.Context, session *concurrency.Sessio
 			closeSession(session) // close the latest session.
 		}
 		if err != nil && errors.Cause(err) != ctx.Err() { // only send non-ctx.Err() error
-			e.l.Error("", zap.Error(err))
 			e.ech <- err
 		}
 	}()
@@ -253,20 +250,25 @@ func (e *Election) newSession(ctx context.Context, retryCnt int) (*concurrency.S
 		session *concurrency.Session
 	)
 	for i := 0; i < retryCnt; i++ {
+		if i > 0 {
+			select {
+			case e.ech <- terror.ErrElectionCampaignFail.Delegate(err, "create a new session"):
+			default:
+			}
+
+			select {
+			case <-time.After(newSessionRetryInterval):
+			case <-ctx.Done():
+				break
+			}
+		}
+
 		// add more options if needed.
 		// NOTE: I think use the client's context is better than something like `concurrency.WithContext(ctx)`,
 		// so we can close the session when the client is still valid.
 		session, err = concurrency.NewSession(e.cli, concurrency.WithTTL(e.sessionTTL))
 		if err == nil || errors.Cause(err) == ctx.Err() {
 			break
-		} else if i%logNewSessionIntervalCnt == 0 {
-			e.l.Warn("fail to create a new session", zap.Error(err))
-		}
-
-		select {
-		case <-time.After(newSessionRetryInterval):
-		case <-ctx.Done():
-			break
 		}
 	}
 	return session, err
diff --git a/pkg/election/election_test.go b/pkg/election/election_test.go
index 946dd6fbb0..d09f5ec037 100644
--- a/pkg/election/election_test.go
+++ b/pkg/election/election_test.go
@@ -153,16 +153,17 @@ func (t *testElectionSuite) TestElection2After1(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(leaderID, Equals, e2.ID())
 
-	// if closing the client when campaigning,
-	// we should get no error because new session will retry until context is done,
-	// and we ignore the context done error.
+	// if closing the client when campaigning, we should get an error
 	wg.Add(1)
 	go func() {
 		defer wg.Done()
 		select {
 		case err2 := <-e2.ErrorNotify():
-			c.Fatalf("should not recieve error %v", err2)
+			c.Assert(terror.ErrElectionCampaignFail.Equal(err2), IsTrue)
+			// the old session is done, but we can't create a new one.
+			c.Assert(err2, ErrorMatches, ".*fail to campaign leader: create a new session.*")
 		case <-time.After(time.Second):
+			c.Fatal("do not receive error for e2")
 		}
 	}()
 	cli.Close() // close the client