ddl: remove checkCleanJobInterval and update a log

pingcap · Jun 19, 2019 · 1a62b70 · 1a62b70
1 parent 70793c3
commit 1a62b70
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 16 deletions.
diff --git a/ddl/ddl.go b/ddl/ddl.go
@@ -450,7 +450,8 @@ func (d *ddl) start(ctx context.Context, ctxPool *pools.ResourcePool) {
 			func() { d.schemaSyncer.StartCleanWork() },
 			func(r interface{}) {
 				if r != nil {
-					logutil.Logger(ddlLogCtx).Error("[ddl] DDL syncer clean worker meet panic", zap.String("ID", d.uuid))
+					logutil.Logger(ddlLogCtx).Error("[ddl] DDL syncer clean worker meet panic",
+						zap.String("ID", d.uuid), zap.Reflect("r", r), zap.Stack("stack trace"))
 					metrics.PanicCounter.WithLabelValues(metrics.LabelDDLSyncer).Inc()
 				}
 			})

diff --git a/ddl/util/syncer.go b/ddl/util/syncer.go
@@ -413,25 +413,23 @@ func (s *schemaVersionSyncer) OwnerCheckAllVersions(ctx context.Context, latestV
 }
 
 const (
-	opDefaultRetryCnt     = 10
-	failedGetTTLLimit     = 20
-	checkCleanJobInterval = 1 * time.Second
-	opDefaultTimeout      = 3 * time.Second
-	opRetryInterval       = 500 * time.Millisecond
+	opDefaultRetryCnt = 10
+	failedGetTTLLimit = 20
+	opDefaultTimeout  = 3 * time.Second
+	opRetryInterval   = 500 * time.Millisecond
 )
 
 // NeededCleanTTL is exported for testing.
 var NeededCleanTTL = int64(-60)
 
 func (s *schemaVersionSyncer) StartCleanWork() {
 	for {
-		time.Sleep(checkCleanJobInterval)
-		if !s.ownerChecker.IsOwner() {
-			continue
-		}
-
 		select {
 		case <-s.notifyCleanExpiredPathsCh:
+			if !s.ownerChecker.IsOwner() {
+				continue
+			}
+
 			for i := 0; i < opDefaultRetryCnt; i++ {
 				childCtx, cancelFunc := context.WithTimeout(context.Background(), opDefaultTimeout)
 				resp, err := s.etcdCli.Leases(childCtx)
@@ -448,7 +446,6 @@ func (s *schemaVersionSyncer) StartCleanWork() {
 			}
 		case <-s.quiteCh:
 			return
-		default:
 		}
 	}
 }

diff --git a/ddl/util/syncer_test.go b/ddl/util/syncer_test.go
@@ -182,15 +182,20 @@ func TestSyncerSimple(t *testing.T) {
 	checkRespKV(t, 1, ttlKey, ttlVal, resp.Kvs...)
 	d.SchemaSyncer().NotifyCleanExpiredPaths()
 	// Make sure the clean worker is done.
-	var isNotified bool
+	notifiedCnt := 1
 	for i := 0; i < 100; i++ {
-		isNotified = d.SchemaSyncer().NotifyCleanExpiredPaths()
+		isNotified := d.SchemaSyncer().NotifyCleanExpiredPaths()
 		if isNotified {
+			notifiedCnt++
+		}
+		// notifyCleanExpiredPathsCh's length is 1,
+		// so when notifiedCnt is 3, we can make sure the clean worker is done at least once.
+		if notifiedCnt == 3 {
 			break
 		}
 		time.Sleep(20 * time.Millisecond)
 	}
-	if !isNotified {
+	if notifiedCnt != 3 {
 		t.Fatal("clean worker don't finish")
 	}
 	// Make sure the ttlKey is removed in etcd.
@@ -228,7 +233,7 @@ func isTimeoutError(err error) bool {
 func checkRespKV(t *testing.T, kvCount int, key, val string,
 	kvs ...*mvccpb.KeyValue) {
 	if len(kvs) != kvCount {
-		t.Fatalf("resp key %s kvs %v length is != 1", key, kvs)
+		t.Fatalf("resp key %s kvs %v length is != %d", key, kvs, kvCount)
 	}
 	if kvCount == 0 {
 		return