statistics: GCStats should not wrongly remove record of an existing table

pkg/statistics/handle/storage/gc.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/pingcap/errors"
+	"github.com/pingcap/failpoint"
 	"github.com/pingcap/tidb/pkg/infoschema"
 	"github.com/pingcap/tidb/pkg/parser/terror"
 	"github.com/pingcap/tidb/pkg/sessionctx"
@@ -91,6 +92,10 @@ func GCStats(
 		return nil
 	}
 
+	failpoint.Inject("mockGCStatsLastTSOffset", func(val failpoint.Value) {
+		offset = uint64(val.(int))
+	})
+
 	// Get the last gc time.
 	gcVer := now - offset
 	lastGC, err := getLastGCTimestamp(sctx)
@@ -266,29 +271,32 @@ func removeDeletedExtendedStats(sctx sessionctx.Context, version uint64) (err er
 }
 
 // gcTableStats GC this table's stats.
+// The GC of a table will be a two-phase process:
+// 1. Delete the column/index's stats from storage. Then other TiDB nodes will be aware that those stats are deleted.
+// 2. Then delete the record in stats_meta.
 func gcTableStats(sctx sessionctx.Context,
 	statsHandler types.StatsHandle,
 	is infoschema.InfoSchema, physicalID int64) error {
+	tbl, ok := statsHandler.TableInfoByID(is, physicalID)
+	if !ok {
+		logutil.BgLogger().Info("remove stats in GC due to dropped table", zap.Int64("table_id", physicalID))
+		return util.WrapTxn(sctx, func(sctx sessionctx.Context) error {
+			return errors.Trace(DeleteTableStatsFromKV(sctx, []int64{physicalID}))
+		})
+	}
 	rows, _, err := util.ExecRows(sctx, "select is_index, hist_id from mysql.stats_histograms where table_id = %?", physicalID)
 	if err != nil {
 		return errors.Trace(err)
 	}
 	// The table has already been deleted in stats and acknowledged to all tidb,
 	// we can safely remove the meta info now.
-	if len(rows) == 0 {
+	if len(rows) == 0 && !ok {
 		_, _, err = util.ExecRows(sctx, "delete from mysql.stats_meta where table_id = %?", physicalID)
 		if err != nil {
 			return errors.Trace(err)
 		}
 		cache.TableRowStatsCache.Invalidate(physicalID)
 	}
-	tbl, ok := statsHandler.TableInfoByID(is, physicalID)
-	if !ok {
-		logutil.BgLogger().Info("remove stats in GC due to dropped table", zap.Int64("table_id", physicalID))
-		return util.WrapTxn(sctx, func(sctx sessionctx.Context) error {
-			return errors.Trace(DeleteTableStatsFromKV(sctx, []int64{physicalID}))
-		})
-	}
 	tblInfo := tbl.Meta()
 	for _, row := range rows {
 		isIndex, histID := row.GetInt64(0), row.GetInt64(1)

pkg/statistics/handle/storage/gc_test.go

@@ -15,9 +15,12 @@
 package storage_test
 
 import (
+	"context"
 	"testing"
 	"time"
 
+	"github.com/pingcap/failpoint"
+	"github.com/pingcap/tidb/pkg/parser/model"
 	"github.com/pingcap/tidb/pkg/sessionctx/variable"
 	"github.com/pingcap/tidb/pkg/testkit"
 	"github.com/pingcap/tidb/pkg/testkit/analyzehelper"
@@ -174,3 +177,26 @@ func TestDeleteAnalyzeJobs(t *testing.T) {
 	rows = testKit.MustQuery("show analyze status").Rows()
 	require.Equal(t, 0, len(rows))
 }
+
+func TestExtremCaseOfGC(t *testing.T) {
+	// This case tests that there's no records in mysql.stats_histograms but this table is not deleted in fact.
+	// We should not delete the record in mysql.stats_meta.
+	store, dom := testkit.CreateMockStoreAndDomain(t)
+	testKit := testkit.NewTestKit(t, store)
+	testKit.MustExec("use test")
+	testKit.MustExec("create table t(a int, b int)")
+	testKit.MustExec("insert into t values (1,2),(3,4)")
+	testKit.MustExec("analyze table t")
+	tbl, err := dom.InfoSchema().TableByName(context.TODO(), model.NewCIStr("test"), model.NewCIStr("t"))
+	require.NoError(t, err)
+	tid := tbl.Meta().ID
+	rs := testKit.MustQuery("select * from mysql.stats_meta where table_id = ?", tid)
+	require.Len(t, rs.Rows(), 1)
+	rs = testKit.MustQuery("select * from mysql.stats_histograms where table_id = ?", tid)
+	require.Len(t, rs.Rows(), 0)
+	h := dom.StatsHandle()
+	failpoint.Enable("github.com/pingcap/tidb/pkg/statistics/handle/storage/mockGCStatsLastTSOffset", `return(0)`)
+	h.GCStats(dom.InfoSchema(), time.Second*3)
+	rs = testKit.MustQuery("select * from mysql.stats_meta where table_id = ?", tid)
+	require.Len(t, rs.Rows(), 1)
+}