diff --git a/internal/logutil/log.go b/internal/logutil/log.go index 0eb789ddb..234f3408a 100644 --- a/internal/logutil/log.go +++ b/internal/logutil/log.go @@ -36,6 +36,7 @@ package logutil import ( "context" + "testing" "github.com/pingcap/log" "go.uber.org/zap" @@ -60,3 +61,11 @@ type ctxLogKeyType struct{} // CtxLogKey is the key to retrieve logger from context. // It can be assigned to another value. var CtxLogKey interface{} = ctxLogKeyType{} + +// AssertWarn panics when in testing mode, and logs a warning msg otherwise. +func AssertWarn(logger *zap.Logger, msg string, fields ...zap.Field) { + if testing.Testing() { + logger.Panic(msg, fields...) + } + logger.Warn(msg, fields...) +} diff --git a/tikv/kv.go b/tikv/kv.go index ac189a8c1..aea5c28d7 100644 --- a/tikv/kv.go +++ b/tikv/kv.go @@ -44,7 +44,6 @@ import ( "strings" "sync" "sync/atomic" - "testing" "time" "github.com/opentracing/opentracing-go" @@ -576,11 +575,7 @@ func (s *KVStore) GetMinSafeTS(txnScope string) uint64 { func (s *KVStore) setMinSafeTS(txnScope string, safeTS uint64) { // ensure safeTS is not set to max uint64 if safeTS == math.MaxUint64 { - if testing.Testing() { - panic(fmt.Errorf("unreachable")) - } else { - logutil.BgLogger().Warn("skip setting min-safe-ts to max uint64", zap.String("txnScope", txnScope), zap.Stack("stack")) - } + logutil.AssertWarn(logutil.BgLogger(), "skip setting min-safe-ts to max uint64", zap.String("txnScope", txnScope), zap.Stack("stack")) return } s.minSafeTS.Store(txnScope, safeTS) @@ -623,11 +618,7 @@ func (s *KVStore) getSafeTS(storeID uint64) (bool, uint64) { func (s *KVStore) setSafeTS(storeID, safeTS uint64) { // ensure safeTS is not set to max uint64 if safeTS == math.MaxUint64 { - if testing.Testing() { - panic(fmt.Errorf("unreachable")) - } else { - logutil.BgLogger().Warn("skip setting safe-ts to max uint64", zap.Uint64("storeID", storeID), zap.Stack("stack")) - } + logutil.AssertWarn(logutil.BgLogger(), "skip setting safe-ts to max uint64", zap.Uint64("storeID", storeID), zap.Stack("stack")) return } s.safeTSMap.Store(storeID, safeTS) @@ -646,13 +637,17 @@ func (s *KVStore) updateMinSafeTS(txnScope string, storeIDs []uint64) { ok, safeTS := s.getSafeTS(store) if ok { // safeTS is guaranteed to be less than math.MaxUint64 (by setSafeTS and its callers) - if safeTS < minSafeTS { + if safeTS != 0 && safeTS < minSafeTS { minSafeTS = safeTS } } else { minSafeTS = 0 } } + // if minSafeTS is still math.MaxUint64, that means all store safe ts are 0, then we set minSafeTS to 0. + if minSafeTS == math.MaxUint64 { + minSafeTS = 0 + } s.setMinSafeTS(txnScope, minSafeTS) } diff --git a/tikv/kv_test.go b/tikv/kv_test.go index c6f11f432..c2e4631e2 100644 --- a/tikv/kv_test.go +++ b/tikv/kv_test.go @@ -164,7 +164,7 @@ func (s *testKVSuite) TestMinSafeTsFromStores() { s.Require().Equal(mockClient.tikvSafeTs, ts) } -func (s *testKVSuite) TestMinSafeTsFromStoresWithZeroSafeTs() { +func (s *testKVSuite) TestMinSafeTsFromStoresWithAllZeros() { // ref https://github.com/tikv/client-go/issues/1276 mockClient := newStoreSafeTsMockClient(s) mockClient.tikvSafeTs = 0 @@ -178,6 +178,19 @@ func (s *testKVSuite) TestMinSafeTsFromStoresWithZeroSafeTs() { s.Require().Equal(uint64(0), s.store.GetMinSafeTS(oracle.GlobalTxnScope)) } +func (s *testKVSuite) TestMinSafeTsFromStoresWithSomeZeros() { + // ref https://github.com/tikv/tikv/issues/13675 & https://github.com/tikv/client-go/pull/615 + mockClient := newStoreSafeTsMockClient(s) + mockClient.tiflashSafeTs = 0 + s.store.SetTiKVClient(mockClient) + + s.Eventually(func() bool { + return atomic.LoadInt32(&mockClient.requestCount) >= 4 + }, 15*time.Second, time.Second) + + s.Require().Equal(mockClient.tikvSafeTs, s.store.GetMinSafeTS(oracle.GlobalTxnScope)) +} + func (s *testKVSuite) TestMinSafeTsFromPD() { mockClient := newStoreSafeTsMockClient(s) s.store.SetTiKVClient(mockClient) @@ -229,7 +242,7 @@ func (s *testKVSuite) TestMinSafeTsFromMixed1() { s.Eventually(func() bool { ts := s.store.GetMinSafeTS("z1") s.Require().False(math.MaxUint64 == ts) - return ts == uint64(10) + return ts == uint64(10) && s.store.GetMinSafeTS(oracle.GlobalTxnScope) == uint64(10) }, 15*time.Second, time.Second) s.Require().GreaterOrEqual(atomic.LoadInt32(&mockClient.requestCount), int32(1)) s.Require().Equal(uint64(10), s.store.GetMinSafeTS(oracle.GlobalTxnScope)) @@ -254,7 +267,7 @@ func (s *testKVSuite) TestMinSafeTsFromMixed2() { s.Eventually(func() bool { ts := s.store.GetMinSafeTS("z2") s.Require().False(math.MaxUint64 == ts) - return ts == uint64(10) + return ts == uint64(10) && s.store.GetMinSafeTS(oracle.GlobalTxnScope) == uint64(10) }, 15*time.Second, time.Second) s.Require().GreaterOrEqual(atomic.LoadInt32(&mockClient.requestCount), int32(1)) s.Require().Equal(uint64(10), s.store.GetMinSafeTS(oracle.GlobalTxnScope))