fix: check kill signal against 0 (#1102)

* fix: check kill signal against 0, so that all kill signals will not be ignored.

Signed-off-by: ekexium <eke@fastmail.com>

* refactor: extract method checkKilled()

Signed-off-by: ekexium <eke@fastmail.com>

---------

Signed-off-by: ekexium <eke@fastmail.com>
Co-authored-by: cfzjywxk <lsswxrxr@163.com>

config/retry/backoff.go

@@ -217,10 +217,9 @@ func (b *Backoffer) BackoffWithCfgAndMaxSleep(cfg *Config, maxSleepMs int, err e
 		atomic.AddInt64(&detail.BackoffCount, 1)
 	}
 
-	if b.vars != nil && b.vars.Killed != nil {
-		if atomic.LoadUint32(b.vars.Killed) == 1 {
-			return errors.WithStack(tikverr.ErrQueryInterrupted)
-		}
+	err2 := b.CheckKilled()
+	if err2 != nil {
+		return err2
 	}
 
 	var startTs interface{}
@@ -382,3 +381,17 @@ func (b *Backoffer) longestSleepCfg() (*Config, int) {
 	}
 	return nil, 0
 }
+
+func (b *Backoffer) CheckKilled() error {
+	if b.vars != nil && b.vars.Killed != nil {
+		killed := atomic.LoadUint32(b.vars.Killed)
+		if killed != 0 {
+			logutil.BgLogger().Info(
+				"backoff stops because a killed signal is received",
+				zap.Uint32("signal", killed),
+			)
+			return errors.WithStack(tikverr.ErrQueryInterrupted)
+		}
+	}
+	return nil
+}

integration_tests/2pc_test.go

@@ -2502,3 +2502,13 @@ func (s *testCommitterSuite) TestExtractKeyExistsErr() {
 	s.True(txn.GetMemBuffer().TryLock())
 	txn.GetMemBuffer().Unlock()
 }
+
+func (s *testCommitterSuite) TestKillSignal() {
+	txn := s.begin()
+	err := txn.Set([]byte("key"), []byte("value"))
+	s.Nil(err)
+	var killed uint32 = 2
+	txn.SetVars(kv.NewVariables(&killed))
+	err = txn.Commit(context.Background())
+	s.ErrorContains(err, "query interrupted")
+}

internal/client/retry/backoff.go

@@ -217,23 +217,38 @@ func (b *Backoffer) BackoffWithCfgAndMaxSleep(cfg *Config, maxSleepMs int, err e
 		atomic.AddInt64(&detail.BackoffCount, 1)
 	}
 
-	if b.vars != nil && b.vars.Killed != nil {
-		if atomic.LoadUint32(b.vars.Killed) == 1 {
-			return errors.WithStack(tikverr.ErrQueryInterrupted)
-		}
+	err2 := b.checkKilled()
+	if err2 != nil {
+		return err2
 	}
 
 	var startTs interface{}
 	if ts := b.ctx.Value(TxnStartKey); ts != nil {
 		startTs = ts
 	}
-	logutil.Logger(b.ctx).Debug("retry later",
+	logutil.Logger(b.ctx).Debug(
+		"retry later",
 		zap.Error(err),
 		zap.Int("totalSleep", b.totalSleep),
 		zap.Int("excludedSleep", b.excludedSleep),
 		zap.Int("maxSleep", b.maxSleep),
 		zap.Stringer("type", cfg),
-		zap.Reflect("txnStartTS", startTs))
+		zap.Reflect("txnStartTS", startTs),
+	)
+	return nil
+}
+
+func (b *Backoffer) checkKilled() error {
+	if b.vars != nil && b.vars.Killed != nil {
+		killed := atomic.LoadUint32(b.vars.Killed)
+		if killed != 0 {
+			logutil.BgLogger().Info(
+				"backoff stops because a killed signal is received",
+				zap.Uint32("signal", killed),
+			)
+			return errors.WithStack(tikverr.ErrQueryInterrupted)
+		}
+	}
 	return nil
 }
 

internal/locate/region_request.go

@@ -1477,9 +1477,8 @@ func (s *RegionRequestSender) SendReqCtx(
 		}
 
 		// recheck whether the session/query is killed during the Next()
-		boVars := bo.GetVars()
-		if boVars != nil && boVars.Killed != nil && atomic.LoadUint32(boVars.Killed) == 1 {
-			return nil, nil, retryTimes, errors.WithStack(tikverr.ErrQueryInterrupted)
+		if err2 := bo.CheckKilled(); err2 != nil {
+			return nil, nil, retryTimes, err2
 		}
 		if val, err := util.EvalFailpoint("mockRetrySendReqToRegion"); err == nil {
 			if val.(bool) {

kv/variables.go

@@ -44,6 +44,10 @@ type Variables struct {
 
 	// Pointer to SessionVars.Killed
 	// Killed is a flag to indicate that this query is killed.
+	// This is an enum value rather than a boolean. See sqlkiller.go
+	// in TiDB for its definition.
+	// When its value is 0, it's not killed
+	// When its value is not 0, it's killed, the value indicates concrete reason.
 	Killed *uint32
 }