internal/client: remove the lock for idle connection recycle

internal/client/client.go

@@ -261,9 +261,7 @@ type RPCClient struct {
 	security config.Security
 
 	idleNotify uint32
-	// recycleMu protect the conns from being modified during a connArray is taken out and used.
-	// That means recycleIdleConnArray() will wait until nobody doing sendBatchRequest()
-	recycleMu sync.RWMutex
+
 	// Periodically check whether there is any connection that is idle and then close and remove these connections.
 	// Implement background cleanup.
 	isClosed    bool
@@ -363,22 +361,18 @@ func (c *RPCClient) SendRequest(ctx context.Context, addr string, req *tikvrpc.R
 		ctx = opentracing.ContextWithSpan(ctx, span1)
 	}
 
-	start := time.Now()
 	if atomic.CompareAndSwapUint32(&c.idleNotify, 1, 0) {
 		go c.recycleIdleConnArray()
 	}
 
 	// TiDB will not send batch commands to TiFlash, to resolve the conflict with Batch Cop Request.
 	enableBatch := req.StoreTp != tikvrpc.TiDB && req.StoreTp != tikvrpc.TiFlash
-	c.recycleMu.RLock()
-	defer c.recycleMu.RUnlock()
 	connArray, err := c.getConnArray(addr, enableBatch)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	metrics.TiKVBatchClientRecycle.Observe(time.Since(start).Seconds())
 
-	start = time.Now()
+	start := time.Now()
 	defer func() {
 		stmtExec := ctx.Value(util.ExecDetailsKey)
 		if stmtExec != nil {

internal/client/client_batch.go

@@ -795,8 +795,7 @@ func sendBatchRequest(
 }
 
 func (c *RPCClient) recycleIdleConnArray() {
-	c.recycleMu.Lock()
-	defer c.recycleMu.Unlock()
+	start := time.Now()
 
 	var addrs []string
 	c.RLock()
@@ -816,8 +815,13 @@ func (c *RPCClient) recycleIdleConnArray() {
 				zap.String("target", addr))
 		}
 		c.Unlock()
+
+		// NOTE, There is a risk that an idle connection become active again, and here is
+		// a race condition someone is using the connection and it's closed here.
 		if conn != nil {
 			conn.Close()
 		}
 	}
+
+	metrics.TiKVBatchClientRecycle.Observe(time.Since(start).Seconds())
 }