diff --git a/Makefile b/Makefile index 66b3ba0686917..ba8c742782900 100644 --- a/Makefile +++ b/Makefile @@ -408,10 +408,6 @@ bazel_coverage_test: failpoint-enable bazel_ci_prepare --build_event_json_file=bazel_1.json --@io_bazel_rules_go//go/config:cover_format=go_cover \ -- //... -//cmd/... -//tests/graceshutdown/... \ -//tests/globalkilltest/... -//tests/readonlytest/... -//br/pkg/task:task_test -//tests/realtikvtest/... - bazel $(BAZEL_GLOBAL_CONFIG) coverage $(BAZEL_CMD_CONFIG) \ - --build_event_json_file=bazel_2.json --@io_bazel_rules_go//go/config:cover_format=go_cover --define gotags=featuretag \ - -- //... -//cmd/... -//tests/graceshutdown/... \ - -//tests/globalkilltest/... -//tests/readonlytest/... -//br/pkg/task:task_test -//tests/realtikvtest/... bazel_build: bazel_ci_prepare mkdir -p bin diff --git a/br/pkg/conn/conn.go b/br/pkg/conn/conn.go index fff775bf1c1d7..157b9cdf794c9 100644 --- a/br/pkg/conn/conn.go +++ b/br/pkg/conn/conn.go @@ -191,7 +191,6 @@ func NewMgr( return nil, errors.Trace(err) } // we must check tidb(tikv version) any time after concurrent ddl feature implemented in v6.2. - // when tidb < 6.2 we need set EnableConcurrentDDL false to make ddl works. // we will keep this check until 7.0, which allow the breaking changes. // NOTE: must call it after domain created! // FIXME: remove this check in v7.0 diff --git a/br/pkg/lightning/backend/local/BUILD.bazel b/br/pkg/lightning/backend/local/BUILD.bazel index c034e6bdb2b3c..9524ab5febc2b 100644 --- a/br/pkg/lightning/backend/local/BUILD.bazel +++ b/br/pkg/lightning/backend/local/BUILD.bazel @@ -103,6 +103,7 @@ go_test( "//br/pkg/lightning/glue", "//br/pkg/lightning/log", "//br/pkg/lightning/mydump", + "//br/pkg/lightning/worker", "//br/pkg/membuf", "//br/pkg/mock", "//br/pkg/pdutil", diff --git a/br/pkg/lightning/backend/local/engine_test.go b/br/pkg/lightning/backend/local/engine_test.go index c7ffe04b95285..eae0225bb519a 100644 --- a/br/pkg/lightning/backend/local/engine_test.go +++ b/br/pkg/lightning/backend/local/engine_test.go @@ -31,8 +31,17 @@ import ( "github.com/stretchr/testify/require" ) -func TestIngestSSTWithClosedEngine(t *testing.T) { +func makePebbleDB(t *testing.T, opt *pebble.Options) (*pebble.DB, string) { dir := t.TempDir() + db, err := pebble.Open(path.Join(dir, "test"), opt) + require.NoError(t, err) + tmpPath := filepath.Join(dir, "test.sst") + err = os.Mkdir(tmpPath, 0o755) + require.NoError(t, err) + return db, tmpPath +} + +func TestIngestSSTWithClosedEngine(t *testing.T) { opt := &pebble.Options{ MemTableSize: 1024 * 1024, MaxConcurrentCompactions: 16, @@ -41,11 +50,7 @@ func TestIngestSSTWithClosedEngine(t *testing.T) { DisableWAL: true, ReadOnly: false, } - db, err := pebble.Open(filepath.Join(dir, "test"), opt) - require.NoError(t, err) - tmpPath := filepath.Join(dir, "test.sst") - err = os.Mkdir(tmpPath, 0o755) - require.NoError(t, err) + db, tmpPath := makePebbleDB(t, opt) _, engineUUID := backend.MakeUUID("ww", 0) engineCtx, cancel := context.WithCancel(context.Background()) diff --git a/br/pkg/lightning/backend/local/local.go b/br/pkg/lightning/backend/local/local.go index e32606207082e..cc88fd6a89483 100644 --- a/br/pkg/lightning/backend/local/local.go +++ b/br/pkg/lightning/backend/local/local.go @@ -91,6 +91,7 @@ const ( gRPCKeepAliveTime = 10 * time.Minute gRPCKeepAliveTimeout = 5 * time.Minute gRPCBackOffMaxDelay = 10 * time.Minute + writeStallSleepTime = 10 * time.Second // The max ranges count in a batch to split and scatter. maxBatchSplitRanges = 4096 @@ -381,6 +382,12 @@ type local struct { encBuilder backend.EncodingBuilder targetInfoGetter backend.TargetInfoGetter + + // When TiKV is in normal mode, ingesting too many SSTs will cause TiKV write stall. + // To avoid this, we should check write stall before ingesting SSTs. Note that, we + // must check both leader node and followers in client side, because followers will + // not check write stall as long as ingest command is accepted by leader. + shouldCheckWriteStall bool } func openDuplicateDB(storeDir string) (*pebble.DB, error) { @@ -503,6 +510,7 @@ func NewLocalBackend( logger: log.FromContext(ctx), encBuilder: NewEncodingBuilder(ctx), targetInfoGetter: NewTargetInfoGetter(tls, g, cfg.TiDB.PdAddr), + shouldCheckWriteStall: cfg.Cron.SwitchMode.Duration == 0, } if m, ok := metric.FromContext(ctx); ok { local.metrics = m @@ -1146,6 +1154,25 @@ func (local *local) Ingest(ctx context.Context, metas []*sst.SSTMeta, region *sp return resp, errors.Trace(err) } + if local.shouldCheckWriteStall { + for { + maybeWriteStall, err := local.checkWriteStall(ctx, region) + if err != nil { + return nil, err + } + if !maybeWriteStall { + break + } + log.FromContext(ctx).Warn("ingest maybe cause write stall, sleep and retry", + zap.Duration("duration", writeStallSleepTime)) + select { + case <-time.After(writeStallSleepTime): + case <-ctx.Done(): + return nil, errors.Trace(ctx.Err()) + } + } + } + req := &sst.MultiIngestRequest{ Context: reqCtx, Ssts: metas, @@ -1154,6 +1181,23 @@ func (local *local) Ingest(ctx context.Context, metas []*sst.SSTMeta, region *sp return resp, errors.Trace(err) } +func (local *local) checkWriteStall(ctx context.Context, region *split.RegionInfo) (bool, error) { + for _, peer := range region.Region.GetPeers() { + cli, err := local.getImportClient(ctx, peer.StoreId) + if err != nil { + return false, errors.Trace(err) + } + resp, err := cli.MultiIngest(ctx, &sst.MultiIngestRequest{}) + if err != nil { + return false, errors.Trace(err) + } + if resp.Error != nil && resp.Error.ServerIsBusy != nil { + return true, nil + } + } + return false, nil +} + func splitRangeBySizeProps(fullRange Range, sizeProps *sizeProperties, sizeLimit int64, keysLimit int64) []Range { ranges := make([]Range, 0, sizeProps.totalSize/uint64(sizeLimit)) curSize := uint64(0) diff --git a/br/pkg/lightning/backend/local/local_test.go b/br/pkg/lightning/backend/local/local_test.go index 1a399552becf9..22d6403d0a1df 100644 --- a/br/pkg/lightning/backend/local/local_test.go +++ b/br/pkg/lightning/backend/local/local_test.go @@ -18,10 +18,10 @@ import ( "bytes" "context" "encoding/binary" + "fmt" "io" "math" "math/rand" - "os" "path/filepath" "sort" "strings" @@ -42,6 +42,7 @@ import ( "github.com/pingcap/tidb/br/pkg/lightning/backend/kv" "github.com/pingcap/tidb/br/pkg/lightning/common" "github.com/pingcap/tidb/br/pkg/lightning/log" + "github.com/pingcap/tidb/br/pkg/lightning/worker" "github.com/pingcap/tidb/br/pkg/membuf" "github.com/pingcap/tidb/br/pkg/pdutil" "github.com/pingcap/tidb/br/pkg/restore/split" @@ -237,8 +238,6 @@ func TestRangeProperties(t *testing.T) { } func TestRangePropertiesWithPebble(t *testing.T) { - dir := t.TempDir() - sizeDistance := uint64(500) keysDistance := uint64(20) opt := &pebble.Options{ @@ -259,8 +258,7 @@ func TestRangePropertiesWithPebble(t *testing.T) { }, }, } - db, err := pebble.Open(filepath.Join(dir, "test"), opt) - require.NoError(t, err) + db, _ := makePebbleDB(t, opt) defer db.Close() // local collector @@ -277,7 +275,7 @@ func TestRangePropertiesWithPebble(t *testing.T) { key := make([]byte, 8) valueLen := rand.Intn(50) binary.BigEndian.PutUint64(key, uint64(i*100+j)) - err = wb.Set(key, value[:valueLen], writeOpt) + err := wb.Set(key, value[:valueLen], writeOpt) require.NoError(t, err) err = collector.Add(pebble.InternalKey{UserKey: key, Trailer: pebble.InternalKeyKindSet}, value[:valueLen]) require.NoError(t, err) @@ -304,7 +302,6 @@ func TestRangePropertiesWithPebble(t *testing.T) { } func testLocalWriter(t *testing.T, needSort bool, partitialSort bool) { - dir := t.TempDir() opt := &pebble.Options{ MemTableSize: 1024 * 1024, MaxConcurrentCompactions: 16, @@ -313,12 +310,8 @@ func testLocalWriter(t *testing.T, needSort bool, partitialSort bool) { DisableWAL: true, ReadOnly: false, } - db, err := pebble.Open(filepath.Join(dir, "test"), opt) - require.NoError(t, err) + db, tmpPath := makePebbleDB(t, opt) defer db.Close() - tmpPath := filepath.Join(dir, "test.sst") - err = os.Mkdir(tmpPath, 0o755) - require.NoError(t, err) _, engineUUID := backend.MakeUUID("ww", 0) engineCtx, cancel := context.WithCancel(context.Background()) @@ -564,7 +557,6 @@ func (i testIngester) ingest([]*sstMeta) error { } func TestLocalIngestLoop(t *testing.T) { - dir := t.TempDir() opt := &pebble.Options{ MemTableSize: 1024 * 1024, MaxConcurrentCompactions: 16, @@ -573,18 +565,14 @@ func TestLocalIngestLoop(t *testing.T) { DisableWAL: true, ReadOnly: false, } - db, err := pebble.Open(filepath.Join(dir, "test"), opt) - require.NoError(t, err) + db, tmpPath := makePebbleDB(t, opt) defer db.Close() - tmpPath := filepath.Join(dir, "test.sst") - err = os.Mkdir(tmpPath, 0o755) - require.NoError(t, err) _, engineUUID := backend.MakeUUID("ww", 0) engineCtx, cancel := context.WithCancel(context.Background()) f := Engine{ db: db, UUID: engineUUID, - sstDir: "", + sstDir: tmpPath, ctx: engineCtx, cancel: cancel, sstMetasChan: make(chan metaOrFlush, 64), @@ -637,7 +625,7 @@ func TestLocalIngestLoop(t *testing.T) { wg.Wait() f.mutex.RLock() - err = f.flushEngineWithoutLock(engineCtx) + err := f.flushEngineWithoutLock(engineCtx) require.NoError(t, err) f.mutex.RUnlock() @@ -732,7 +720,6 @@ func TestFilterOverlapRange(t *testing.T) { } func testMergeSSTs(t *testing.T, kvs [][]common.KvPair, meta *sstMeta) { - dir := t.TempDir() opt := &pebble.Options{ MemTableSize: 1024 * 1024, MaxConcurrentCompactions: 16, @@ -741,12 +728,8 @@ func testMergeSSTs(t *testing.T, kvs [][]common.KvPair, meta *sstMeta) { DisableWAL: true, ReadOnly: false, } - db, err := pebble.Open(filepath.Join(dir, "test"), opt) - require.NoError(t, err) + db, tmpPath := makePebbleDB(t, opt) defer db.Close() - tmpPath := filepath.Join(dir, "test.sst") - err = os.Mkdir(tmpPath, 0o755) - require.NoError(t, err) _, engineUUID := backend.MakeUUID("ww", 0) engineCtx, cancel := context.WithCancel(context.Background()) @@ -837,49 +820,90 @@ func TestMergeSSTsDuplicated(t *testing.T) { type mockPdClient struct { pd.Client - stores []*metapb.Store + stores []*metapb.Store + regions []*pd.Region } func (c *mockPdClient) GetAllStores(ctx context.Context, opts ...pd.GetStoreOption) ([]*metapb.Store, error) { return c.stores, nil } +func (c *mockPdClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*pd.Region, error) { + return c.regions, nil +} + type mockGrpcErr struct{} func (e mockGrpcErr) GRPCStatus() *status.Status { - return status.New(codes.Unimplemented, "unimplmented") + return status.New(codes.Unimplemented, "unimplemented") } func (e mockGrpcErr) Error() string { - return "unimplmented" + return "unimplemented" } type mockImportClient struct { sst.ImportSSTClient store *metapb.Store + resp *sst.IngestResponse err error retry int cnt int multiIngestCheckFn func(s *metapb.Store) bool + apiInvokeRecorder map[string][]uint64 +} + +func newMockImportClient() *mockImportClient { + return &mockImportClient{ + multiIngestCheckFn: func(s *metapb.Store) bool { + return true + }, + } } func (c *mockImportClient) MultiIngest(context.Context, *sst.MultiIngestRequest, ...grpc.CallOption) (*sst.IngestResponse, error) { defer func() { c.cnt++ }() - if c.cnt < c.retry && c.err != nil { - return nil, c.err + if c.apiInvokeRecorder != nil { + c.apiInvokeRecorder["MultiIngest"] = append(c.apiInvokeRecorder["MultiIngest"], c.store.GetId()) + } + if c.cnt < c.retry && (c.err != nil || c.resp != nil) { + return c.resp, c.err } if !c.multiIngestCheckFn(c.store) { return nil, mockGrpcErr{} } - return nil, nil + return &sst.IngestResponse{}, nil +} + +type mockWriteClient struct { + sst.ImportSST_WriteClient + writeResp *sst.WriteResponse +} + +func (m mockWriteClient) Send(request *sst.WriteRequest) error { + return nil +} + +func (m mockWriteClient) CloseAndRecv() (*sst.WriteResponse, error) { + return m.writeResp, nil +} + +func (c *mockImportClient) Write(ctx context.Context, opts ...grpc.CallOption) (sst.ImportSST_WriteClient, error) { + if c.apiInvokeRecorder != nil { + c.apiInvokeRecorder["Write"] = append(c.apiInvokeRecorder["Write"], c.store.GetId()) + } + return mockWriteClient{writeResp: &sst.WriteResponse{Metas: []*sst.SSTMeta{ + {}, {}, {}, + }}}, nil } type mockImportClientFactory struct { - stores []*metapb.Store - createClientFn func(store *metapb.Store) sst.ImportSSTClient + stores []*metapb.Store + createClientFn func(store *metapb.Store) sst.ImportSSTClient + apiInvokeRecorder map[string][]uint64 } func (f *mockImportClientFactory) Create(_ context.Context, storeID uint64) (sst.ImportSSTClient, error) { @@ -888,7 +912,7 @@ func (f *mockImportClientFactory) Create(_ context.Context, storeID uint64) (sst return f.createClientFn(store), nil } } - return nil, errors.New("store not found") + return nil, fmt.Errorf("store %d not found", storeID) } func (f *mockImportClientFactory) Close() {} @@ -1220,3 +1244,75 @@ func TestLocalIsRetryableTiKVWriteError(t *testing.T) { require.True(t, l.isRetryableImportTiKVError(io.EOF)) require.True(t, l.isRetryableImportTiKVError(errors.Trace(io.EOF))) } + +func TestCheckPeersBusy(t *testing.T) { + ctx := context.Background() + pdCli := &mockPdClient{} + pdCtl := &pdutil.PdController{} + pdCtl.SetPDClient(pdCli) + + keys := [][]byte{[]byte(""), []byte("a"), []byte("b"), []byte("")} + splitCli := initTestSplitClient3Replica(keys, nil) + apiInvokeRecorder := map[string][]uint64{} + serverIsBusyResp := &sst.IngestResponse{ + Error: &errorpb.Error{ + ServerIsBusy: &errorpb.ServerIsBusy{}, + }} + + createTimeStore12 := 0 + local := &local{ + pdCtl: pdCtl, + splitCli: splitCli, + importClientFactory: &mockImportClientFactory{ + stores: []*metapb.Store{ + // region ["", "a") is not used, skip (1, 2, 3) + {Id: 11}, {Id: 12}, {Id: 13}, // region ["a", "b") + {Id: 21}, {Id: 22}, {Id: 23}, // region ["b", "") + }, + createClientFn: func(store *metapb.Store) sst.ImportSSTClient { + importCli := newMockImportClient() + importCli.store = store + importCli.apiInvokeRecorder = apiInvokeRecorder + if store.Id == 12 { + createTimeStore12++ + // the second time to checkWriteStall + if createTimeStore12 == 2 { + importCli.retry = 1 + importCli.resp = serverIsBusyResp + } + } + return importCli + }, + }, + logger: log.L(), + ingestConcurrency: worker.NewPool(ctx, 1, "ingest"), + writeLimiter: noopStoreWriteLimiter{}, + bufferPool: membuf.NewPool(), + supportMultiIngest: true, + shouldCheckWriteStall: true, + } + + db, tmpPath := makePebbleDB(t, nil) + _, engineUUID := backend.MakeUUID("ww", 0) + engineCtx, cancel := context.WithCancel(context.Background()) + f := &Engine{ + db: db, + UUID: engineUUID, + sstDir: tmpPath, + ctx: engineCtx, + cancel: cancel, + sstMetasChan: make(chan metaOrFlush, 64), + keyAdapter: noopKeyAdapter{}, + logger: log.L(), + } + err := f.db.Set([]byte("a"), []byte("a"), nil) + require.NoError(t, err) + err = f.db.Set([]byte("b"), []byte("b"), nil) + require.NoError(t, err) + err = local.writeAndIngestByRange(ctx, f, []byte("a"), []byte("c"), 0, 0) + require.NoError(t, err) + + require.Equal(t, []uint64{11, 12, 13, 21, 22, 23}, apiInvokeRecorder["Write"]) + // store 12 has a follower busy, so it will cause region peers (11, 12, 13) retry once + require.Equal(t, []uint64{11, 12, 11, 12, 13, 11, 21, 22, 23, 21}, apiInvokeRecorder["MultiIngest"]) +} diff --git a/br/pkg/lightning/backend/local/localhelper_test.go b/br/pkg/lightning/backend/local/localhelper_test.go index 6cbf7f2f14808..023fade304fae 100644 --- a/br/pkg/lightning/backend/local/localhelper_test.go +++ b/br/pkg/lightning/backend/local/localhelper_test.go @@ -47,7 +47,7 @@ func init() { splitRetryTimes = 2 } -type testClient struct { +type testSplitClient struct { mu sync.RWMutex stores map[uint64]*metapb.Store regions map[uint64]*split.RegionInfo @@ -57,17 +57,17 @@ type testClient struct { hook clientHook } -func newTestClient( +func newTestSplitClient( stores map[uint64]*metapb.Store, regions map[uint64]*split.RegionInfo, nextRegionID uint64, hook clientHook, -) *testClient { +) *testSplitClient { regionsInfo := &pdtypes.RegionTree{} for _, regionInfo := range regions { regionsInfo.SetRegion(pdtypes.NewRegionInfo(regionInfo.Region, regionInfo.Leader)) } - return &testClient{ + return &testSplitClient{ stores: stores, regions: regions, regionsInfo: regionsInfo, @@ -77,17 +77,17 @@ func newTestClient( } // ScatterRegions scatters regions in a batch. -func (c *testClient) ScatterRegions(ctx context.Context, regionInfo []*split.RegionInfo) error { +func (c *testSplitClient) ScatterRegions(ctx context.Context, regionInfo []*split.RegionInfo) error { return nil } -func (c *testClient) GetAllRegions() map[uint64]*split.RegionInfo { +func (c *testSplitClient) GetAllRegions() map[uint64]*split.RegionInfo { c.mu.RLock() defer c.mu.RUnlock() return c.regions } -func (c *testClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) { +func (c *testSplitClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Store, error) { c.mu.RLock() defer c.mu.RUnlock() store, ok := c.stores[storeID] @@ -97,19 +97,18 @@ func (c *testClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Stor return store, nil } -func (c *testClient) GetRegion(ctx context.Context, key []byte) (*split.RegionInfo, error) { +func (c *testSplitClient) GetRegion(ctx context.Context, key []byte) (*split.RegionInfo, error) { c.mu.RLock() defer c.mu.RUnlock() for _, region := range c.regions { - if bytes.Compare(key, region.Region.StartKey) >= 0 && - (len(region.Region.EndKey) == 0 || bytes.Compare(key, region.Region.EndKey) < 0) { + if bytes.Compare(key, region.Region.StartKey) >= 0 && beforeEnd(key, region.Region.EndKey) { return region, nil } } return nil, errors.Errorf("region not found: key=%s", string(key)) } -func (c *testClient) GetRegionByID(ctx context.Context, regionID uint64) (*split.RegionInfo, error) { +func (c *testSplitClient) GetRegionByID(ctx context.Context, regionID uint64) (*split.RegionInfo, error) { c.mu.RLock() defer c.mu.RUnlock() region, ok := c.regions[regionID] @@ -119,7 +118,7 @@ func (c *testClient) GetRegionByID(ctx context.Context, regionID uint64) (*split return region, nil } -func (c *testClient) SplitRegion( +func (c *testSplitClient) SplitRegion( ctx context.Context, regionInfo *split.RegionInfo, key []byte, @@ -130,7 +129,7 @@ func (c *testClient) SplitRegion( splitKey := codec.EncodeBytes([]byte{}, key) for _, region := range c.regions { if bytes.Compare(splitKey, region.Region.StartKey) >= 0 && - (len(region.Region.EndKey) == 0 || bytes.Compare(splitKey, region.Region.EndKey) < 0) { + beforeEnd(splitKey, region.Region.EndKey) { target = region } } @@ -159,7 +158,7 @@ func (c *testClient) SplitRegion( return newRegion, nil } -func (c *testClient) BatchSplitRegionsWithOrigin( +func (c *testSplitClient) BatchSplitRegionsWithOrigin( ctx context.Context, regionInfo *split.RegionInfo, keys [][]byte, ) (*split.RegionInfo, []*split.RegionInfo, error) { c.mu.Lock() @@ -234,24 +233,24 @@ func (c *testClient) BatchSplitRegionsWithOrigin( return target, newRegions, err } -func (c *testClient) BatchSplitRegions( +func (c *testSplitClient) BatchSplitRegions( ctx context.Context, regionInfo *split.RegionInfo, keys [][]byte, ) ([]*split.RegionInfo, error) { _, newRegions, err := c.BatchSplitRegionsWithOrigin(ctx, regionInfo, keys) return newRegions, err } -func (c *testClient) ScatterRegion(ctx context.Context, regionInfo *split.RegionInfo) error { +func (c *testSplitClient) ScatterRegion(ctx context.Context, regionInfo *split.RegionInfo) error { return nil } -func (c *testClient) GetOperator(ctx context.Context, regionID uint64) (*pdpb.GetOperatorResponse, error) { +func (c *testSplitClient) GetOperator(ctx context.Context, regionID uint64) (*pdpb.GetOperatorResponse, error) { return &pdpb.GetOperatorResponse{ Header: new(pdpb.ResponseHeader), }, nil } -func (c *testClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*split.RegionInfo, error) { +func (c *testSplitClient) ScanRegions(ctx context.Context, key, endKey []byte, limit int) ([]*split.RegionInfo, error) { if c.hook != nil { key, endKey, limit = c.hook.BeforeScanRegions(ctx, key, endKey, limit) } @@ -272,19 +271,19 @@ func (c *testClient) ScanRegions(ctx context.Context, key, endKey []byte, limit return regions, err } -func (c *testClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) (r pdtypes.Rule, err error) { +func (c *testSplitClient) GetPlacementRule(ctx context.Context, groupID, ruleID string) (r pdtypes.Rule, err error) { return } -func (c *testClient) SetPlacementRule(ctx context.Context, rule pdtypes.Rule) error { +func (c *testSplitClient) SetPlacementRule(ctx context.Context, rule pdtypes.Rule) error { return nil } -func (c *testClient) DeletePlacementRule(ctx context.Context, groupID, ruleID string) error { +func (c *testSplitClient) DeletePlacementRule(ctx context.Context, groupID, ruleID string) error { return nil } -func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error { +func (c *testSplitClient) SetStoresLabel(ctx context.Context, stores []uint64, labelKey, labelValue string) error { return nil } @@ -305,7 +304,7 @@ func cloneRegion(region *split.RegionInfo) *split.RegionInfo { // For keys ["", "aay", "bba", "bbh", "cca", ""], the key ranges of // regions are [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ). -func initTestClient(keys [][]byte, hook clientHook) *testClient { +func initTestSplitClient(keys [][]byte, hook clientHook) *testSplitClient { peers := make([]*metapb.Peer, 1) peers[0] = &metapb.Peer{ Id: 1, @@ -329,13 +328,56 @@ func initTestClient(keys [][]byte, hook clientHook) *testClient { EndKey: endKey, RegionEpoch: &metapb.RegionEpoch{ConfVer: 1, Version: 1}, }, + Leader: peers[0], } } stores := make(map[uint64]*metapb.Store) stores[1] = &metapb.Store{ Id: 1, } - return newTestClient(stores, regions, uint64(len(keys)), hook) + return newTestSplitClient(stores, regions, uint64(len(keys)), hook) +} + +// initTestSplitClient3Replica will create a client that each region has 3 replicas, and their IDs and StoreIDs are +// (1, 2, 3), (11, 12, 13), ... +// For keys ["", "aay", "bba", "bbh", "cca", ""], the key ranges of +// region ranges are [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ). +func initTestSplitClient3Replica(keys [][]byte, hook clientHook) *testSplitClient { + regions := make(map[uint64]*split.RegionInfo) + stores := make(map[uint64]*metapb.Store) + for i := uint64(1); i < uint64(len(keys)); i++ { + startKey := keys[i-1] + if len(startKey) != 0 { + startKey = codec.EncodeBytes([]byte{}, startKey) + } + endKey := keys[i] + if len(endKey) != 0 { + endKey = codec.EncodeBytes([]byte{}, endKey) + } + baseID := (i-1)*10 + 1 + peers := make([]*metapb.Peer, 3) + for j := 0; j < 3; j++ { + peers[j] = &metapb.Peer{ + Id: baseID + uint64(j), + StoreId: baseID + uint64(j), + } + } + + regions[baseID] = &split.RegionInfo{ + Region: &metapb.Region{ + Id: baseID, + Peers: peers, + StartKey: startKey, + EndKey: endKey, + RegionEpoch: &metapb.RegionEpoch{ConfVer: 1, Version: 1}, + }, + Leader: peers[0], + } + stores[baseID] = &metapb.Store{ + Id: baseID, + } + } + return newTestSplitClient(stores, regions, uint64(len(keys)), hook) } func checkRegionRanges(t *testing.T, regions []*split.RegionInfo, keys [][]byte) { @@ -376,7 +418,7 @@ func (h *noopHook) AfterScanRegions(res []*split.RegionInfo, err error) ([]*spli type batchSplitHook interface { setup(t *testing.T) func() - check(t *testing.T, cli *testClient) + check(t *testing.T, cli *testSplitClient) } type defaultHook struct{} @@ -392,7 +434,7 @@ func (d defaultHook) setup(t *testing.T) func() { } } -func (d defaultHook) check(t *testing.T, cli *testClient) { +func (d defaultHook) check(t *testing.T, cli *testSplitClient) { // so with a batch split size of 4, there will be 7 time batch split // 1. region: [aay, bba), keys: [b, ba, bb] // 2. region: [bbh, cca), keys: [bc, bd, be, bf] @@ -414,7 +456,7 @@ func doTestBatchSplitRegionByRanges(ctx context.Context, t *testing.T, hook clie defer deferFunc() keys := [][]byte{[]byte(""), []byte("aay"), []byte("bba"), []byte("bbh"), []byte("cca"), []byte("")} - client := initTestClient(keys, hook) + client := initTestSplitClient(keys, hook) local := &local{ splitCli: client, g: glue.NewExternalTiDBGlue(nil, mysql.ModeNone), @@ -479,7 +521,7 @@ func (h batchSizeHook) setup(t *testing.T) func() { } } -func (h batchSizeHook) check(t *testing.T, cli *testClient) { +func (h batchSizeHook) check(t *testing.T, cli *testSplitClient) { // so with a batch split key size of 6, there will be 9 time batch split // 1. region: [aay, bba), keys: [b, ba, bb] // 2. region: [bbh, cca), keys: [bc, bd, be] @@ -583,7 +625,7 @@ func TestSplitAndScatterRegionInBatches(t *testing.T) { defer deferFunc() keys := [][]byte{[]byte(""), []byte("a"), []byte("b"), []byte("")} - client := initTestClient(keys, nil) + client := initTestSplitClient(keys, nil) local := &local{ splitCli: client, g: glue.NewExternalTiDBGlue(nil, mysql.ModeNone), @@ -670,7 +712,7 @@ func doTestBatchSplitByRangesWithClusteredIndex(t *testing.T, hook clientHook) { keys = append(keys, key) } keys = append(keys, tableEndKey, []byte("")) - client := initTestClient(keys, hook) + client := initTestSplitClient(keys, hook) local := &local{ splitCli: client, g: glue.NewExternalTiDBGlue(nil, mysql.ModeNone), diff --git a/br/pkg/version/BUILD.bazel b/br/pkg/version/BUILD.bazel index 8171081ae5df1..7a15014e378e6 100644 --- a/br/pkg/version/BUILD.bazel +++ b/br/pkg/version/BUILD.bazel @@ -10,7 +10,6 @@ go_library( "//br/pkg/logutil", "//br/pkg/utils", "//br/pkg/version/build", - "//sessionctx/variable", "//util/engine", "@com_github_coreos_go_semver//semver", "@com_github_pingcap_errors//:errors", @@ -29,7 +28,6 @@ go_test( flaky = True, deps = [ "//br/pkg/version/build", - "//sessionctx/variable", "@com_github_coreos_go_semver//semver", "@com_github_data_dog_go_sqlmock//:go-sqlmock", "@com_github_pingcap_kvproto//pkg/metapb", diff --git a/br/pkg/version/version.go b/br/pkg/version/version.go index c49e3d1ada923..9cb974d48e13f 100644 --- a/br/pkg/version/version.go +++ b/br/pkg/version/version.go @@ -18,7 +18,6 @@ import ( "github.com/pingcap/tidb/br/pkg/logutil" "github.com/pingcap/tidb/br/pkg/utils" "github.com/pingcap/tidb/br/pkg/version/build" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util/engine" pd "github.com/tikv/pd/client" "go.uber.org/zap" @@ -166,9 +165,7 @@ func CheckVersionForDDL(s *metapb.Store, tikvVersion *semver.Version) error { // use tikvVersion instead of tidbVersion since br doesn't have mysql client to connect tidb. requireVersion := semver.New("6.2.0-alpha") if tikvVersion.Compare(*requireVersion) < 0 { - log.Info("detected the old version of tidb cluster. set enable concurrent ddl to false") - variable.EnableConcurrentDDL.Store(false) - return nil + return errors.Errorf("detected the old version of tidb cluster, require: >= 6.2.0, but got %s", tikvVersion.String()) } return nil } diff --git a/br/pkg/version/version_test.go b/br/pkg/version/version_test.go index 1fc654b3990b6..927eeee119d5b 100644 --- a/br/pkg/version/version_test.go +++ b/br/pkg/version/version_test.go @@ -13,7 +13,6 @@ import ( "github.com/coreos/go-semver/semver" "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/br/pkg/version/build" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/stretchr/testify/require" pd "github.com/tikv/pd/client" ) @@ -321,50 +320,40 @@ func TestCheckClusterVersion(t *testing.T) { mock.getAllStores = func() []*metapb.Store { return []*metapb.Store{{Version: "v6.4.0"}} } - originVal := variable.EnableConcurrentDDL.Load() err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) require.NoError(t, err) - require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) } { mock.getAllStores = func() []*metapb.Store { return []*metapb.Store{{Version: "v6.2.0"}} } - originVal := variable.EnableConcurrentDDL.Load() err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) require.NoError(t, err) - require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) } { mock.getAllStores = func() []*metapb.Store { return []*metapb.Store{{Version: "v6.2.0-alpha"}} } - originVal := variable.EnableConcurrentDDL.Load() err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) require.NoError(t, err) - require.Equal(t, originVal, variable.EnableConcurrentDDL.Load()) } { mock.getAllStores = func() []*metapb.Store { return []*metapb.Store{{Version: "v6.1.0"}} } - variable.EnableConcurrentDDL.Store(true) err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) - require.NoError(t, err) - require.False(t, variable.EnableConcurrentDDL.Load()) + require.Error(t, err) } { mock.getAllStores = func() []*metapb.Store { return []*metapb.Store{{Version: "v5.4.0"}} } - variable.EnableConcurrentDDL.Store(true) err := CheckClusterVersion(context.Background(), &mock, CheckVersionForDDL) - require.NoError(t, err) - require.False(t, variable.EnableConcurrentDDL.Load()) + require.Error(t, err) } } diff --git a/ddl/column.go b/ddl/column.go index 3bbe7417fd7b5..04af3f1714a1d 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -806,7 +806,7 @@ func doReorgWorkForModifyColumnMultiSchema(w *worker, d *ddlCtx, t *meta.Meta, j func doReorgWorkForModifyColumn(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table, oldCol, changingCol *model.ColumnInfo, changingIdxs []*model.IndexInfo) (done bool, ver int64, err error) { job.ReorgMeta.ReorgTp = model.ReorgTypeTxn - rh := newReorgHandler(t, w.sess, w.concurrentDDL) + rh := newReorgHandler(t, w.sess) dbInfo, err := t.GetDatabase(job.SchemaID) if err != nil { return false, ver, errors.Trace(err) @@ -1654,6 +1654,15 @@ func updateColumnDefaultValue(d *ddlCtx, t *meta.Meta, job *model.Job, newCol *m job.State = model.JobStateCancelled return ver, infoschema.ErrColumnNotExists.GenWithStackByArgs(newCol.Name, tblInfo.Name) } + + if hasDefaultValue, _, err := checkColumnDefaultValue(newContext(d.store), table.ToColumn(oldCol.Clone()), newCol.DefaultValue); err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } else if !hasDefaultValue { + job.State = model.JobStateCancelled + return ver, dbterror.ErrInvalidDefaultValue.GenWithStackByArgs(newCol.Name) + } + // The newCol's offset may be the value of the old schema version, so we can't use newCol directly. oldCol.DefaultValue = newCol.DefaultValue oldCol.DefaultValueBit = newCol.DefaultValueBit diff --git a/ddl/concurrentddltest/BUILD.bazel b/ddl/concurrentddltest/BUILD.bazel deleted file mode 100644 index 82e2adf1fe9c2..0000000000000 --- a/ddl/concurrentddltest/BUILD.bazel +++ /dev/null @@ -1,26 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_test") - -go_test( - name = "concurrentddltest_test", - timeout = "moderate", - srcs = [ - "main_test.go", - "switch_test.go", - ], - flaky = True, - race = "on", - shard_count = 2, - deps = [ - "//config", - "//ddl", - "//kv", - "//meta", - "//sessionctx/variable", - "//testkit", - "//testkit/testsetup", - "//util", - "@com_github_stretchr_testify//require", - "@org_uber_go_atomic//:atomic", - "@org_uber_go_goleak//:goleak", - ], -) diff --git a/ddl/concurrentddltest/main_test.go b/ddl/concurrentddltest/main_test.go deleted file mode 100644 index 4ab7e96eab2ae..0000000000000 --- a/ddl/concurrentddltest/main_test.go +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2022 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package concurrentddltest - -import ( - "testing" - "time" - - "github.com/pingcap/tidb/config" - "github.com/pingcap/tidb/ddl" - "github.com/pingcap/tidb/testkit/testsetup" - "go.uber.org/goleak" -) - -func TestMain(m *testing.M) { - testsetup.SetupForCommonTest() - - config.UpdateGlobal(func(conf *config.Config) { - conf.TiKVClient.AsyncCommit.SafeWindow = 0 - conf.TiKVClient.AsyncCommit.AllowedClockDrift = 0 - }) - - ddl.SetWaitTimeWhenErrorOccurred(time.Microsecond) - - opts := []goleak.Option{ - goleak.IgnoreTopFunction("github.com/golang/glog.(*loggingT).flushDaemon"), - goleak.IgnoreTopFunction("github.com/lestrrat-go/httprc.runFetchWorker"), - goleak.IgnoreTopFunction("go.etcd.io/etcd/client/pkg/v3/logutil.(*MergeLogger).outputLoop"), - goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"), - } - - goleak.VerifyTestMain(m, opts...) -} diff --git a/ddl/concurrentddltest/switch_test.go b/ddl/concurrentddltest/switch_test.go deleted file mode 100644 index 6cd26811008e6..0000000000000 --- a/ddl/concurrentddltest/switch_test.go +++ /dev/null @@ -1,149 +0,0 @@ -// Copyright 2022 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package concurrentddltest - -import ( - "context" - "fmt" - "math/rand" - "testing" - "time" - - "github.com/pingcap/tidb/kv" - "github.com/pingcap/tidb/meta" - "github.com/pingcap/tidb/sessionctx/variable" - "github.com/pingcap/tidb/testkit" - "github.com/pingcap/tidb/util" - "github.com/stretchr/testify/require" - "go.uber.org/atomic" -) - -func TestConcurrentDDLSwitch(t *testing.T) { - store := testkit.CreateMockStore(t) - - type table struct { - columnIdx int - indexIdx int - } - - var tables []*table - tblCount := 20 - for i := 0; i < tblCount; i++ { - tables = append(tables, &table{1, 0}) - } - - tk := testkit.NewTestKit(t, store) - tk.MustExec("use test") - tk.MustExec("set global tidb_enable_metadata_lock=0") - tk.MustExec("set @@global.tidb_ddl_reorg_worker_cnt=1") - tk.MustExec("set @@global.tidb_ddl_reorg_batch_size=32") - - for i := range tables { - tk.MustExec(fmt.Sprintf("create table t%d (col0 int)", i)) - for j := 0; j < 1000; j++ { - tk.MustExec(fmt.Sprintf("insert into t%d values (%d)", i, j)) - } - } - - ddls := make([]string, 0, tblCount) - ddlCount := 100 - for i := 0; i < ddlCount; i++ { - tblIdx := rand.Intn(tblCount) - if rand.Intn(2) == 0 { - ddls = append(ddls, fmt.Sprintf("alter table t%d add index idx%d (col0)", tblIdx, tables[tblIdx].indexIdx)) - tables[tblIdx].indexIdx++ - } else { - ddls = append(ddls, fmt.Sprintf("alter table t%d add column col%d int", tblIdx, tables[tblIdx].columnIdx)) - tables[tblIdx].columnIdx++ - } - } - - c := atomic.NewInt32(0) - ch := make(chan struct{}) - go func() { - var wg util.WaitGroupWrapper - for i := range ddls { - wg.Add(1) - go func(idx int) { - tk := testkit.NewTestKit(t, store) - tk.MustExec("use test") - tk.MustExec(ddls[idx]) - c.Add(1) - wg.Done() - }(i) - } - wg.Wait() - ch <- struct{}{} - }() - - // sleep 2s to make sure the ddl jobs is into table. - time.Sleep(2 * time.Second) - ticker := time.NewTicker(time.Second) - count := 0 - done := false - for !done { - select { - case <-ch: - done = true - case <-ticker.C: - var b bool - var err error - err = kv.RunInNewTxn(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), store, false, func(ctx context.Context, txn kv.Transaction) error { - b, err = meta.NewMeta(txn).IsConcurrentDDL() - return err - }) - require.NoError(t, err) - rs, err := testkit.NewTestKit(t, store).Exec(fmt.Sprintf("set @@global.tidb_enable_concurrent_ddl=%t", !b)) - if rs != nil { - require.NoError(t, rs.Close()) - } - if err == nil { - count++ - if b { - tk := testkit.NewTestKit(t, store) - tk.Session().GetSessionVars().MemQuotaQuery = -1 - tk.MustQuery("select count(*) from mysql.tidb_ddl_job").Check(testkit.Rows("0")) - tk.MustQuery("select count(*) from mysql.tidb_ddl_reorg").Check(testkit.Rows("0")) - } - } - } - } - - require.Equal(t, int32(ddlCount), c.Load()) - require.Greater(t, count, 0) - - tk = testkit.NewTestKit(t, store) - tk.Session().GetSessionVars().MemQuotaQuery = -1 - tk.MustExec("use test") - for i, tbl := range tables { - tk.MustQuery(fmt.Sprintf("select count(*) from information_schema.columns where TABLE_SCHEMA = 'test' and TABLE_NAME = 't%d'", i)).Check(testkit.Rows(fmt.Sprintf("%d", tbl.columnIdx))) - tk.MustExec(fmt.Sprintf("admin check table t%d", i)) - for j := 0; j < tbl.indexIdx; j++ { - tk.MustExec(fmt.Sprintf("admin check index t%d idx%d", i, j)) - } - } -} - -func TestConcurrentDDLSwitchWithMDL(t *testing.T) { - if !variable.EnableConcurrentDDL.Load() { - t.Skip("skip test if concurrent DDL is disabled") - } - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - tk.MustGetErrMsg("set global tidb_enable_concurrent_ddl=off", "can not disable concurrent ddl when metadata lock is enabled") - tk.MustExec("set global tidb_enable_metadata_lock=0") - tk.MustExec("set global tidb_enable_concurrent_ddl=off") - tk.MustExec("create table test.t(a int)") -} diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 442b797a54dbc..cc9cc657fdc6f 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -2373,6 +2373,43 @@ func TestSqlFunctionsInGeneratedColumns(t *testing.T) { tk.MustExec("create table t (a int, b int as ((a)))") } +func TestSchemaNameAndTableNameInGeneratedExpr(t *testing.T) { + store := testkit.CreateMockStore(t, mockstore.WithDDLChecker()) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("create database if not exists test") + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + + tk.MustExec("create table t(a int, b int as (lower(test.t.a)))") + tk.MustQuery("show create table t").Check(testkit.Rows("t CREATE TABLE `t` (\n" + + " `a` int(11) DEFAULT NULL,\n" + + " `b` int(11) GENERATED ALWAYS AS (lower(`a`)) VIRTUAL\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + + tk.MustExec("drop table t") + tk.MustExec("create table t(a int)") + tk.MustExec("alter table t add column b int as (lower(test.t.a))") + tk.MustQuery("show create table t").Check(testkit.Rows("t CREATE TABLE `t` (\n" + + " `a` int(11) DEFAULT NULL,\n" + + " `b` int(11) GENERATED ALWAYS AS (lower(`a`)) VIRTUAL\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + + tk.MustGetErrCode("alter table t add index idx((lower(test.t1.a)))", errno.ErrBadField) + + tk.MustExec("drop table t") + tk.MustGetErrCode("create table t(a int, b int as (lower(test1.t.a)))", errno.ErrWrongDBName) + + tk.MustExec("create table t(a int)") + tk.MustGetErrCode("alter table t add column b int as (lower(test.t1.a))", errno.ErrWrongTableName) + + tk.MustExec("alter table t add column c int") + tk.MustGetErrCode("alter table t modify column c int as (test.t1.a + 1) stored", errno.ErrWrongTableName) + + tk.MustExec("alter table t add column d int as (lower(test.T.a))") + tk.MustExec("alter table t add column e int as (lower(Test.t.a))") +} + func TestParserIssue284(t *testing.T) { store := testkit.CreateMockStore(t, mockstore.WithDDLChecker()) diff --git a/ddl/db_test.go b/ddl/db_test.go index d48bacd3947c0..7d1e829b3db61 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -20,6 +20,7 @@ import ( "math" "strconv" "strings" + "sync" "testing" "time" @@ -1789,3 +1790,37 @@ func TestHashPartitionAddColumn(t *testing.T) { dom.DDL().SetHook(hook) tk.MustExec("alter table t add column c int") } + +func TestSetInvalidDefaultValueAfterModifyColumn(t *testing.T) { + store, dom := testkit.CreateMockStoreAndDomain(t) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t(a int, b int)") + + var wg sync.WaitGroup + var checkErr error + one := false + hook := &ddl.TestDDLCallback{Do: dom} + hook.OnJobRunBeforeExported = func(job *model.Job) { + if job.SchemaState != model.StateDeleteOnly { + return + } + if !one { + one = true + } else { + return + } + wg.Add(1) + go func() { + tk2 := testkit.NewTestKit(t, store) + tk2.MustExec("use test") + _, checkErr = tk2.Exec("alter table t alter column a set default 1") + wg.Done() + }() + } + dom.DDL().SetHook(hook) + tk.MustExec("alter table t modify column a text(100)") + wg.Wait() + require.EqualError(t, checkErr, "[ddl:1101]BLOB/TEXT/JSON column 'a' can't have a default value") +} diff --git a/ddl/ddl.go b/ddl/ddl.go index 1e1b38eeb77bb..4cbdcfde9eeef 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -252,10 +252,6 @@ type DDL interface { GetInfoSchemaWithInterceptor(ctx sessionctx.Context) infoschema.InfoSchema // DoDDLJob does the DDL job, it's exported for test. DoDDLJob(ctx sessionctx.Context, job *model.Job) error - // MoveJobFromQueue2Table move existing DDLs from queue to table. - MoveJobFromQueue2Table(bool) error - // MoveJobFromTable2Queue move existing DDLs from table to queue. - MoveJobFromTable2Queue() error } type limitJobTask struct { @@ -270,7 +266,6 @@ type ddl struct { limitJobCh chan *limitJobTask *ddlCtx - workers map[workerType]*worker sessPool *sessionPool delRangeMgr delRangeManager enableTiFlashPoll *atomicutil.Bool @@ -624,7 +619,6 @@ func newDDL(ctx context.Context, options ...Option) *ddl { // Register functions for enable/disable ddl when changing system variable `tidb_enable_ddl`. variable.EnableDDL = d.EnableDDL variable.DisableDDL = d.DisableDDL - variable.SwitchConcurrentDDL = d.SwitchConcurrentDDL variable.SwitchMDL = d.SwitchMDL return d @@ -656,7 +650,7 @@ func (d *ddl) newDeleteRangeManager(mock bool) delRangeManager { func (d *ddl) prepareWorkers4ConcurrencyDDL() { workerFactory := func(tp workerType) func() (pools.Resource, error) { return func() (pools.Resource, error) { - wk := newWorker(d.ctx, tp, d.sessPool, d.delRangeMgr, d.ddlCtx, true) + wk := newWorker(d.ctx, tp, d.sessPool, d.delRangeMgr, d.ddlCtx) sessForJob, err := d.sessPool.get() if err != nil { return nil, err @@ -679,23 +673,6 @@ func (d *ddl) prepareWorkers4ConcurrencyDDL() { d.wg.Run(d.startDispatchLoop) } -func (d *ddl) prepareWorkers4legacyDDL() { - d.workers = make(map[workerType]*worker, 2) - d.workers[generalWorker] = newWorker(d.ctx, generalWorker, d.sessPool, d.delRangeMgr, d.ddlCtx, false) - d.workers[addIdxWorker] = newWorker(d.ctx, addIdxWorker, d.sessPool, d.delRangeMgr, d.ddlCtx, false) - for _, worker := range d.workers { - worker.wg.Add(1) - w := worker - go w.start(d.ddlCtx) - - metrics.DDLCounter.WithLabelValues(fmt.Sprintf("%s_%s", metrics.CreateDDL, worker.String())).Inc() - - // When the start function is called, we will send a fake job to let worker - // checks owner firstly and try to find whether a job exists and run. - asyncNotify(worker.ddlJobCh) - } -} - // Start implements DDL.Start interface. func (d *ddl) Start(ctxPool *pools.ResourcePool) error { logutil.BgLogger().Info("[ddl] start DDL", zap.String("ID", d.uuid), zap.Bool("runWorker", config.GetGlobalConfig().Instance.TiDBEnableDDL.Load())) @@ -713,7 +690,6 @@ func (d *ddl) Start(ctxPool *pools.ResourcePool) error { d.delRangeMgr = d.newDeleteRangeManager(ctxPool == nil) d.prepareWorkers4ConcurrencyDDL() - d.prepareWorkers4legacyDDL() if config.TableLockEnabled() { d.wg.Add(1) @@ -799,9 +775,6 @@ func (d *ddl) close() { d.generalDDLWorkerPool.close() } - for _, worker := range d.workers { - worker.Close() - } // d.delRangeMgr using sessions from d.sessPool. // Put it before d.sessPool.close to reduce the time spent by d.sessPool.close. if d.delRangeMgr != nil { @@ -921,24 +894,10 @@ func (d *ddl) asyncNotifyWorker(job *model.Job) { if !config.GetGlobalConfig().Instance.TiDBEnableDDL.Load() { return } - if variable.EnableConcurrentDDL.Load() { - if d.isOwner() { - asyncNotify(d.ddlJobCh) - } else { - d.asyncNotifyByEtcd(addingDDLJobConcurrent, job) - } + if d.isOwner() { + asyncNotify(d.ddlJobCh) } else { - var worker *worker - if job.MayNeedReorg() { - worker = d.workers[addIdxWorker] - } else { - worker = d.workers[generalWorker] - } - if d.ownerManager.IsOwner() { - asyncNotify(worker.ddlJobCh) - } else { - d.asyncNotifyByEtcd(worker.addingDDLJobKey, job) - } + d.asyncNotifyByEtcd(addingDDLJobConcurrent, job) } } @@ -1055,7 +1014,7 @@ func (d *ddl) DoDDLJob(ctx sessionctx.Context, job *model.Job) error { continue } sessVars.StmtCtx.DDLJobID = 0 // Avoid repeat. - errs, err := CancelJobs(se, d.store, []int64{jobID}) + errs, err := CancelJobs(se, []int64{jobID}) d.sessPool.put(se) if len(errs) > 0 { logutil.BgLogger().Warn("error canceling DDL job", zap.Error(errs[0])) @@ -1182,55 +1141,12 @@ func (d *ddl) startCleanDeadTableLock() { } } -// SwitchConcurrentDDL changes the DDL to concurrent DDL if toConcurrentDDL is true, otherwise, queue based DDL. -func (d *ddl) SwitchConcurrentDDL(toConcurrentDDL bool) error { - if !d.isOwner() { - return kv.RunInNewTxn(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), d.store, true, func(ctx context.Context, txn kv.Transaction) error { - isConcurrentDDL, err := meta.NewMeta(txn).IsConcurrentDDL() - if err != nil { - return err - } - if isConcurrentDDL != toConcurrentDDL { - return errors.New("please set it on the DDL owner node") - } - return nil - }) - } - - if variable.EnableMDL.Load() && !toConcurrentDDL { - return errors.New("can not disable concurrent ddl when metadata lock is enabled") - } - - ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) - defer cancel() - d.waiting.Store(true) - defer d.waiting.Store(false) - if err := d.wait4Switch(ctx); err != nil { - return err - } - - var err error - if toConcurrentDDL { - err = d.MoveJobFromQueue2Table(false) - } else { - err = d.MoveJobFromTable2Queue() - } - if err == nil { - variable.EnableConcurrentDDL.Store(toConcurrentDDL) - logutil.BgLogger().Info("[ddl] SwitchConcurrentDDL", zap.Bool("toConcurrentDDL", toConcurrentDDL)) - } else { - logutil.BgLogger().Warn("[ddl] SwitchConcurrentDDL", zap.Bool("toConcurrentDDL", toConcurrentDDL), zap.Error(err)) - } - return err -} - // SwitchMDL enables MDL or disable DDL. func (d *ddl) SwitchMDL(enable bool) error { ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) defer cancel() - // Disable MDL for test. - if enable && !variable.DefTiDBEnableConcurrentDDL { + if enable { sql := fmt.Sprintf("UPDATE HIGH_PRIORITY %[1]s.%[2]s SET VARIABLE_VALUE = %[4]d WHERE VARIABLE_NAME = '%[3]s'", mysql.SystemDB, mysql.GlobalVariablesTable, variable.TiDBEnableMDL, 0) sess, err := d.sessPool.get() @@ -1288,23 +1204,6 @@ func (d *ddl) SwitchMDL(enable bool) error { return nil } -func (d *ddl) wait4Switch(ctx context.Context) error { - for { - select { - case <-ctx.Done(): - return ctx.Err() - default: - } - d.runningJobs.RLock() - if len(d.runningJobs.ids) == 0 { - d.runningJobs.RUnlock() - return nil - } - d.runningJobs.RUnlock() - time.Sleep(time.Second * 1) - } -} - // RecoverInfo contains information needed by DDL.RecoverTable. type RecoverInfo struct { SchemaID int64 @@ -1419,13 +1318,8 @@ func GetDDLInfo(s sessionctx.Context) (*Info, error) { } t := meta.NewMeta(txn) info.Jobs = make([]*model.Job, 0, 2) - enable := variable.EnableConcurrentDDL.Load() var generalJob, reorgJob *model.Job - if enable { - generalJob, reorgJob, err = get2JobsFromTable(sess) - } else { - generalJob, reorgJob, err = get2JobsFromQueue(t) - } + generalJob, reorgJob, err = get2JobsFromTable(sess) if err != nil { return nil, errors.Trace(err) } @@ -1446,7 +1340,7 @@ func GetDDLInfo(s sessionctx.Context) (*Info, error) { return info, nil } - _, info.ReorgHandle, _, _, err = newReorgHandler(t, sess, enable).GetDDLReorgHandle(reorgJob) + _, info.ReorgHandle, _, _, err = newReorgHandler(t, sess).GetDDLReorgHandle(reorgJob) if err != nil { if meta.ErrDDLReorgElementNotExist.Equal(err) { return info, nil @@ -1457,19 +1351,6 @@ func GetDDLInfo(s sessionctx.Context) (*Info, error) { return info, nil } -func get2JobsFromQueue(t *meta.Meta) (*model.Job, *model.Job, error) { - generalJob, err := t.GetDDLJobByIdx(0) - if err != nil { - return nil, nil, errors.Trace(err) - } - reorgJob, err := t.GetDDLJobByIdx(0, meta.AddIndexJobListKey) - if err != nil { - return nil, nil, errors.Trace(err) - } - - return generalJob, reorgJob, nil -} - func get2JobsFromTable(sess *session) (*model.Job, *model.Job, error) { var generalJob, reorgJob *model.Job jobs, err := getJobsBySQL(sess, JobTable, "not reorg order by job_id limit 1") @@ -1491,82 +1372,8 @@ func get2JobsFromTable(sess *session) (*model.Job, *model.Job, error) { } // CancelJobs cancels the DDL jobs. -func CancelJobs(se sessionctx.Context, store kv.Storage, ids []int64) (errs []error, err error) { - if variable.EnableConcurrentDDL.Load() { - return cancelConcurrencyJobs(se, ids) - } - - err = kv.RunInNewTxn(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), store, true, func(ctx context.Context, txn kv.Transaction) error { - errs, err = cancelLegacyJobs(txn, ids) - return err - }) - return -} - -func cancelLegacyJobs(txn kv.Transaction, ids []int64) ([]error, error) { - if len(ids) == 0 { - return nil, nil - } - - errs := make([]error, len(ids)) - t := meta.NewMeta(txn) - generalJobs, err := getDDLJobsInQueue(t, meta.DefaultJobListKey) - if err != nil { - return nil, errors.Trace(err) - } - addIdxJobs, err := getDDLJobsInQueue(t, meta.AddIndexJobListKey) - if err != nil { - return nil, errors.Trace(err) - } - jobs := append(generalJobs, addIdxJobs...) - jobsMap := make(map[int64]int) - for i, id := range ids { - jobsMap[id] = i - } - for j, job := range jobs { - i, ok := jobsMap[job.ID] - if !ok { - logutil.BgLogger().Debug("the job that needs to be canceled isn't equal to current job", - zap.Int64("need to canceled job ID", job.ID), - zap.Int64("current job ID", job.ID)) - continue - } - delete(jobsMap, job.ID) - // These states can't be cancelled. - if job.IsDone() || job.IsSynced() { - errs[i] = dbterror.ErrCancelFinishedDDLJob.GenWithStackByArgs(job.ID) - continue - } - // If the state is rolling back, it means the work is cleaning the data after cancelling the job. - if job.IsCancelled() || job.IsRollingback() || job.IsRollbackDone() { - continue - } - if !job.IsRollbackable() { - errs[i] = dbterror.ErrCannotCancelDDLJob.GenWithStackByArgs(job.ID) - continue - } - - job.State = model.JobStateCancelling - // Make sure RawArgs isn't overwritten. - err := json.Unmarshal(job.RawArgs, &job.Args) - if err != nil { - errs[i] = errors.Trace(err) - continue - } - if j >= len(generalJobs) { - offset := int64(j - len(generalJobs)) - err = t.UpdateDDLJob(offset, job, true, meta.AddIndexJobListKey) - } else { - err = t.UpdateDDLJob(int64(j), job, true) - } - if err != nil { - errs[i] = errors.Trace(err) - } - } - for id, i := range jobsMap { - errs[i] = dbterror.ErrDDLJobNotFound.GenWithStackByArgs(id) - } - return errs, nil +func CancelJobs(se sessionctx.Context, ids []int64) (errs []error, err error) { + return cancelConcurrencyJobs(se, ids) } // cancelConcurrencyJobs cancels the DDL jobs that are in the concurrent state. @@ -1645,45 +1452,9 @@ func cancelConcurrencyJobs(se sessionctx.Context, ids []int64) ([]error, error) return errs, nil } -func getDDLJobsInQueue(t *meta.Meta, jobListKey meta.JobListKeyType) ([]*model.Job, error) { - cnt, err := t.DDLJobQueueLen(jobListKey) - if err != nil { - return nil, errors.Trace(err) - } - jobs := make([]*model.Job, cnt) - for i := range jobs { - jobs[i], err = t.GetDDLJobByIdx(int64(i), jobListKey) - if err != nil { - return nil, errors.Trace(err) - } - } - return jobs, nil -} - // GetAllDDLJobs get all DDL jobs and sorts jobs by job.ID. func GetAllDDLJobs(sess sessionctx.Context, t *meta.Meta) ([]*model.Job, error) { - if variable.EnableConcurrentDDL.Load() { - return getJobsBySQL(newSession(sess), JobTable, "1 order by job_id") - } - - return getDDLJobs(t) -} - -// getDDLJobs get all DDL jobs and sorts jobs by job.ID. -func getDDLJobs(t *meta.Meta) ([]*model.Job, error) { - generalJobs, err := getDDLJobsInQueue(t, meta.DefaultJobListKey) - if err != nil { - return nil, errors.Trace(err) - } - addIdxJobs, err := getDDLJobsInQueue(t, meta.AddIndexJobListKey) - if err != nil { - return nil, errors.Trace(err) - } - jobs := append(generalJobs, addIdxJobs...) - slices.SortFunc(jobs, func(i, j *model.Job) bool { - return i.ID < j.ID - }) - return jobs, nil + return getJobsBySQL(newSession(sess), JobTable, "1 order by job_id") } // MaxHistoryJobs is exported for testing. @@ -1873,19 +1644,11 @@ func GetHistoryJobByID(sess sessionctx.Context, id int64) (*model.Job, error) { return job, errors.Trace(err) } -// AddHistoryDDLJobForTest used for test. -func AddHistoryDDLJobForTest(sess sessionctx.Context, t *meta.Meta, job *model.Job, updateRawArgs bool) error { - return AddHistoryDDLJob(newSession(sess), t, job, updateRawArgs, variable.EnableConcurrentDDL.Load()) -} - // AddHistoryDDLJob record the history job. -func AddHistoryDDLJob(sess *session, t *meta.Meta, job *model.Job, updateRawArgs bool, concurrentDDL bool) error { - if concurrentDDL { - // only add history job into table if it is concurrent DDL. - err := addHistoryDDLJob2Table(sess, job, updateRawArgs) - if err != nil { - logutil.BgLogger().Info("[ddl] failed to add DDL job to history table", zap.Error(err)) - } +func AddHistoryDDLJob(sess *session, t *meta.Meta, job *model.Job, updateRawArgs bool) error { + err := addHistoryDDLJob2Table(sess, job, updateRawArgs) + if err != nil { + logutil.BgLogger().Info("[ddl] failed to add DDL job to history table", zap.Error(err)) } // we always add history DDL job to job list at this moment. return t.AddHistoryDDLJob(job, updateRawArgs) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 2ae8a46134d4b..7cd65b47b170a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1082,7 +1082,7 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o var sb strings.Builder restoreFlags := format.RestoreStringSingleQuotes | format.RestoreKeyWordLowercase | format.RestoreNameBackQuotes | - format.RestoreSpacesAroundBinaryOperation + format.RestoreSpacesAroundBinaryOperation | format.RestoreWithoutSchemaName | format.RestoreWithoutTableName restoreCtx := format.NewRestoreCtx(restoreFlags, &sb) for _, v := range colDef.Options { @@ -1142,7 +1142,10 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o } col.GeneratedExprString = sb.String() col.GeneratedStored = v.Stored - _, dependColNames := findDependedColumnNames(colDef) + _, dependColNames, err := findDependedColumnNames(model.NewCIStr(""), model.NewCIStr(""), colDef) + if err != nil { + return nil, nil, errors.Trace(err) + } col.Dependences = dependColNames case ast.ColumnOptionCollate: if field_types.HasCharset(colDef.Tp) { @@ -1561,7 +1564,7 @@ func IsAutoRandomColumnID(tblInfo *model.TableInfo, colID int64) bool { return false } -func checkGeneratedColumn(ctx sessionctx.Context, colDefs []*ast.ColumnDef) error { +func checkGeneratedColumn(ctx sessionctx.Context, schemaName model.CIStr, tableName model.CIStr, colDefs []*ast.ColumnDef) error { var colName2Generation = make(map[string]columnGenerationInDDL, len(colDefs)) var exists bool var autoIncrementColumn string @@ -1576,7 +1579,10 @@ func checkGeneratedColumn(ctx sessionctx.Context, colDefs []*ast.ColumnDef) erro if containsColumnOption(colDef, ast.ColumnOptionAutoIncrement) { exists, autoIncrementColumn = true, colDef.Name.Name.L } - generated, depCols := findDependedColumnNames(colDef) + generated, depCols, err := findDependedColumnNames(schemaName, tableName, colDef) + if err != nil { + return errors.Trace(err) + } if !generated { colName2Generation[colDef.Name.Name.L] = columnGenerationInDDL{ position: i, @@ -2094,7 +2100,7 @@ func CheckTableInfoValidWithStmt(ctx sessionctx.Context, tbInfo *model.TableInfo func checkTableInfoValidWithStmt(ctx sessionctx.Context, tbInfo *model.TableInfo, s *ast.CreateTableStmt) (err error) { // All of these rely on the AST structure of expressions, which were // lost in the model (got serialized into strings). - if err := checkGeneratedColumn(ctx, s.Cols); err != nil { + if err := checkGeneratedColumn(ctx, s.Table.Schema, tbInfo.Name, s.Cols); err != nil { return errors.Trace(err) } @@ -3672,7 +3678,10 @@ func CreateNewColumn(ctx sessionctx.Context, ti ast.Ident, schema *model.DBInfo, return nil, dbterror.ErrUnsupportedOnGeneratedColumn.GenWithStackByArgs("Adding generated stored column through ALTER TABLE") } - _, dependColNames := findDependedColumnNames(specNewColumn) + _, dependColNames, err := findDependedColumnNames(schema.Name, t.Meta().Name, specNewColumn) + if err != nil { + return nil, errors.Trace(err) + } if !ctx.GetSessionVars().EnableAutoIncrementInGenerated { if err := checkAutoIncrementRef(specNewColumn.Name.Name.L, dependColNames, t.Meta()); err != nil { return nil, errors.Trace(err) @@ -4485,7 +4494,7 @@ func setColumnComment(ctx sessionctx.Context, col *table.Column, option *ast.Col func processColumnOptions(ctx sessionctx.Context, col *table.Column, options []*ast.ColumnOption) error { var sb strings.Builder restoreFlags := format.RestoreStringSingleQuotes | format.RestoreKeyWordLowercase | format.RestoreNameBackQuotes | - format.RestoreSpacesAroundBinaryOperation + format.RestoreSpacesAroundBinaryOperation | format.RestoreWithoutSchemaName | format.RestoreWithoutSchemaName restoreCtx := format.NewRestoreCtx(restoreFlags, &sb) var hasDefaultValue, setOnUpdateNow bool @@ -4827,7 +4836,7 @@ func GetModifiableColumnJob( } // As same with MySQL, we don't support modifying the stored status for generated columns. - if err = checkModifyGeneratedColumn(sctx, t, col, newCol, specNewColumn, spec.Position); err != nil { + if err = checkModifyGeneratedColumn(sctx, schema.Name, t, col, newCol, specNewColumn, spec.Position); err != nil { return nil, errors.Trace(err) } @@ -6306,7 +6315,7 @@ func BuildHiddenColumnInfo(ctx sessionctx.Context, indexPartSpecifications []*as var sb strings.Builder restoreFlags := format.RestoreStringSingleQuotes | format.RestoreKeyWordLowercase | format.RestoreNameBackQuotes | - format.RestoreSpacesAroundBinaryOperation + format.RestoreSpacesAroundBinaryOperation | format.RestoreWithoutSchemaName | format.RestoreWithoutTableName restoreCtx := format.NewRestoreCtx(restoreFlags, &sb) sb.Reset() err := idxPart.Expr.Restore(restoreCtx) diff --git a/ddl/ddl_api_test.go b/ddl/ddl_api_test.go index f4010015f5456..9f36cc95f806c 100644 --- a/ddl/ddl_api_test.go +++ b/ddl/ddl_api_test.go @@ -115,73 +115,6 @@ func TestGetDDLJobsIsSort(t *testing.T) { require.NoError(t, err) } -func TestGetHistoryDDLJobs(t *testing.T) { - store := testkit.CreateMockStore(t) - - // delete the internal DDL record. - err := kv.RunInNewTxn(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), store, false, func(ctx context.Context, txn kv.Transaction) error { - return meta.NewMeta(txn).ClearAllHistoryJob() - }) - require.NoError(t, err) - testkit.NewTestKit(t, store).MustExec("delete from mysql.tidb_ddl_history") - - tk := testkit.NewTestKit(t, store) - sess := tk.Session() - tk.MustExec("begin") - - txn, err := sess.Txn(true) - require.NoError(t, err) - - m := meta.NewMeta(txn) - cnt := 11 - jobs := make([]*model.Job, cnt) - for i := 0; i < cnt; i++ { - jobs[i] = &model.Job{ - ID: int64(i), - SchemaID: 1, - Type: model.ActionCreateTable, - } - err = ddl.AddHistoryDDLJobForTest(sess, m, jobs[i], true) - require.NoError(t, err) - - historyJobs, err := ddl.GetLastNHistoryDDLJobs(m, ddl.DefNumHistoryJobs) - require.NoError(t, err) - - if i+1 > ddl.MaxHistoryJobs { - require.Len(t, historyJobs, ddl.MaxHistoryJobs) - } else { - require.Len(t, historyJobs, i+1) - } - } - - delta := cnt - ddl.MaxHistoryJobs - historyJobs, err := ddl.GetLastNHistoryDDLJobs(m, ddl.DefNumHistoryJobs) - require.NoError(t, err) - require.Len(t, historyJobs, ddl.MaxHistoryJobs) - - l := len(historyJobs) - 1 - for i, job := range historyJobs { - require.Equal(t, jobs[delta+l-i].ID, job.ID) - require.Equal(t, int64(1), job.SchemaID) - require.Equal(t, model.ActionCreateTable, job.Type) - } - - var historyJobs2 []*model.Job - err = ddl.IterHistoryDDLJobs(txn, func(jobs []*model.Job) (b bool, e error) { - for _, job := range jobs { - historyJobs2 = append(historyJobs2, job) - if len(historyJobs2) == ddl.DefNumHistoryJobs { - return true, nil - } - } - return false, nil - }) - require.NoError(t, err) - require.Equal(t, historyJobs, historyJobs2) - - tk.MustExec("rollback") -} - func TestIsJobRollbackable(t *testing.T) { cases := []struct { tp model.ActionType diff --git a/ddl/ddl_test.go b/ddl/ddl_test.go index a2e75119e4d12..6b210d2445c26 100644 --- a/ddl/ddl_test.go +++ b/ddl/ddl_test.go @@ -19,7 +19,6 @@ import ( "testing" "time" - "github.com/pingcap/failpoint" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" @@ -271,109 +270,6 @@ func TestBuildJobDependence(t *testing.T) { require.NoError(t, err) } -func TestNotifyDDLJob(t *testing.T) { - store := createMockStore(t) - defer func() { - require.NoError(t, store.Close()) - }() - - require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/NoDDLDispatchLoop", `return(true)`)) - defer require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/NoDDLDispatchLoop")) - - getFirstNotificationAfterStartDDL := func(d *ddl) { - select { - case <-d.workers[addIdxWorker].ddlJobCh: - default: - // The notification may be received by the worker. - } - select { - case <-d.workers[generalWorker].ddlJobCh: - default: - // The notification may be received by the worker. - } - - select { - case <-d.ddlJobCh: - default: - } - } - - d, err := testNewDDLAndStart( - context.Background(), - WithStore(store), - WithLease(testLease), - ) - require.NoError(t, err) - defer func() { - require.NoError(t, d.Stop()) - }() - getFirstNotificationAfterStartDDL(d) - // Ensure that the notification is not handled in workers `start` function. - d.cancel() - for _, worker := range d.workers { - worker.Close() - } - - job := &model.Job{ - SchemaID: 1, - TableID: 2, - Type: model.ActionCreateTable, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{}, - } - // Test the notification mechanism of the owner and the server receiving the DDL request on the same TiDB. - // This DDL request is a general DDL job. - d.asyncNotifyWorker(job) - select { - case <-d.workers[generalWorker].ddlJobCh: - case <-d.ddlJobCh: - default: - require.FailNow(t, "do not get the general job notification") - } - // Test the notification mechanism of the owner and the server receiving the DDL request on the same TiDB. - // This DDL request is a add index DDL job. - job.Type = model.ActionAddIndex - d.asyncNotifyWorker(job) - select { - case <-d.workers[addIdxWorker].ddlJobCh: - case <-d.ddlJobCh: - default: - require.FailNow(t, "do not get the add index job notification") - } - - // Test the notification mechanism that the owner and the server receiving the DDL request are not on the same TiDB. - // And the etcd client is nil. - d1, err := testNewDDLAndStart( - context.Background(), - WithStore(store), - WithLease(testLease), - ) - require.NoError(t, err) - defer func() { - require.NoError(t, d1.Stop()) - }() - getFirstNotificationAfterStartDDL(d1) - // Ensure that the notification is not handled by worker's "start". - d1.cancel() - for _, worker := range d1.workers { - worker.Close() - } - d1.ownerManager.RetireOwner() - d1.asyncNotifyWorker(job) - job.Type = model.ActionCreateTable - d1.asyncNotifyWorker(job) - require.False(t, d1.OwnerManager().IsOwner()) - select { - case <-d1.workers[addIdxWorker].ddlJobCh: - require.FailNow(t, "should not get the add index job notification") - case <-d1.workers[generalWorker].ddlJobCh: - require.FailNow(t, "should not get the general job notification") - case <-d1.ddlJobCh: - require.FailNow(t, "should not get the job notification") - default: - } -} - func TestError(t *testing.T) { kvErrs := []*terror.Error{ dbterror.ErrDDLJobNotFound, diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index a760cb598129b..7843fac34a69e 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -97,8 +97,6 @@ type worker struct { logCtx context.Context lockSeqNum bool - concurrentDDL bool - *ddlCtx } @@ -123,7 +121,7 @@ func NewJobContext() *JobContext { } } -func newWorker(ctx context.Context, tp workerType, sessPool *sessionPool, delRangeMgr delRangeManager, dCtx *ddlCtx, concurrentDDL bool) *worker { +func newWorker(ctx context.Context, tp workerType, sessPool *sessionPool, delRangeMgr delRangeManager, dCtx *ddlCtx) *worker { worker := &worker{ id: ddlWorkerID.Add(1), tp: tp, @@ -132,7 +130,6 @@ func newWorker(ctx context.Context, tp workerType, sessPool *sessionPool, delRan ddlCtx: dCtx, sessPool: sessPool, delRangeManager: delRangeMgr, - concurrentDDL: concurrentDDL, } worker.addingDDLJobKey = addingDDLJobPrefix + worker.typeStr() worker.logCtx = logutil.WithKeyValue(context.Background(), "worker", worker.String()) @@ -165,59 +162,6 @@ func (w *worker) Close() { logutil.Logger(w.logCtx).Info("[ddl] DDL worker closed", zap.Duration("take time", time.Since(startTime))) } -// start is used for async online schema changing, it will try to become the owner firstly, -// then wait or pull the job queue to handle a schema change job. -func (w *worker) start(d *ddlCtx) { - logutil.Logger(w.logCtx).Info("[ddl] start DDL worker") - defer w.wg.Done() - defer tidbutil.Recover( - metrics.LabelDDLWorker, - fmt.Sprintf("DDL ID %s, %s start", d.uuid, w), - nil, true, - ) - - // We use 4 * lease time to check owner's timeout, so here, we will update owner's status - // every 2 * lease time. If lease is 0, we will use default 1s. - // But we use etcd to speed up, normally it takes less than 1s now, so we use 1s as the max value. - checkTime := chooseLeaseTime(2*d.lease, 1*time.Second) - - ticker := time.NewTicker(checkTime) - defer ticker.Stop() - var notifyDDLJobByEtcdCh clientv3.WatchChan - if d.etcdCli != nil { - notifyDDLJobByEtcdCh = d.etcdCli.Watch(context.Background(), w.addingDDLJobKey) - } - - rewatchCnt := 0 - for { - ok := true - select { - case <-ticker.C: - logutil.Logger(w.logCtx).Debug("[ddl] wait to check DDL status again", zap.Duration("interval", checkTime)) - case <-w.ddlJobCh: - case _, ok = <-notifyDDLJobByEtcdCh: - case <-w.ctx.Done(): - return - } - - if !ok { - logutil.Logger(w.logCtx).Warn("[ddl] start worker watch channel closed", zap.String("watch key", w.addingDDLJobKey)) - notifyDDLJobByEtcdCh = d.etcdCli.Watch(context.Background(), w.addingDDLJobKey) - rewatchCnt++ - if rewatchCnt > 10 { - time.Sleep(time.Duration(rewatchCnt) * time.Second) - } - continue - } - - rewatchCnt = 0 - err := w.handleDDLJobQueue(d) - if err != nil { - logutil.Logger(w.logCtx).Warn("[ddl] handle DDL job failed", zap.Error(err)) - } - } -} - func (d *ddl) asyncNotifyByEtcd(addingDDLJobKey string, job *model.Job) { if d.etcdCli == nil { return @@ -239,37 +183,6 @@ func asyncNotify(ch chan struct{}) { } } -// buildJobDependence sets the curjob's dependency-ID. -// The dependency-job's ID must less than the current job's ID, and we need the largest one in the list. -func buildJobDependence(t *meta.Meta, curJob *model.Job) error { - // Jobs in the same queue are ordered. If we want to find a job's dependency-job, we need to look for - // it from the other queue. So if the job is "ActionAddIndex" job, we need find its dependency-job from DefaultJobList. - jobListKey := meta.DefaultJobListKey - if !curJob.MayNeedReorg() { - jobListKey = meta.AddIndexJobListKey - } - jobs, err := t.GetAllDDLJobsInQueue(jobListKey) - if err != nil { - return errors.Trace(err) - } - - for _, job := range jobs { - if curJob.ID < job.ID { - continue - } - isDependent, err := curJob.IsDependentOn(job) - if err != nil { - return errors.Trace(err) - } - if isDependent { - logutil.BgLogger().Info("[ddl] current DDL job depends on other job", zap.String("currentJob", curJob.String()), zap.String("dependentJob", job.String())) - curJob.DependencyID = job.ID - break - } - } - return nil -} - func (d *ddl) limitDDLJobs() { defer tidbutil.Recover(metrics.LabelDDL, "limitDDLJobs", nil, true) @@ -295,7 +208,7 @@ func (d *ddl) addBatchDDLJobs(tasks []*limitJobTask) { startTime := time.Now() var err error // DDLForce2Queue is a flag to tell DDL worker to always push the job to the DDL queue. - toTable := variable.EnableConcurrentDDL.Load() && !variable.DDLForce2Queue.Load() + toTable := !variable.DDLForce2Queue.Load() if toTable { err = d.addBatchDDLJobs2Table(tasks) } else { @@ -315,6 +228,37 @@ func (d *ddl) addBatchDDLJobs(tasks []*limitJobTask) { } } +// buildJobDependence sets the curjob's dependency-ID. +// The dependency-job's ID must less than the current job's ID, and we need the largest one in the list. +func buildJobDependence(t *meta.Meta, curJob *model.Job) error { + // Jobs in the same queue are ordered. If we want to find a job's dependency-job, we need to look for + // it from the other queue. So if the job is "ActionAddIndex" job, we need find its dependency-job from DefaultJobList. + jobListKey := meta.DefaultJobListKey + if !curJob.MayNeedReorg() { + jobListKey = meta.AddIndexJobListKey + } + jobs, err := t.GetAllDDLJobsInQueue(jobListKey) + if err != nil { + return errors.Trace(err) + } + + for _, job := range jobs { + if curJob.ID < job.ID { + continue + } + isDependent, err := curJob.IsDependentOn(job) + if err != nil { + return errors.Trace(err) + } + if isDependent { + logutil.BgLogger().Info("[ddl] current DDL job depends on other job", zap.String("currentJob", curJob.String()), zap.String("dependentJob", job.String())) + curJob.DependencyID = job.ID + break + } + } + return nil +} + func (d *ddl) addBatchDDLJobs2Queue(tasks []*limitJobTask) error { ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL) return kv.RunInNewTxn(ctx, d.store, true, func(ctx context.Context, txn kv.Transaction) error { @@ -444,13 +388,6 @@ func injectFailPointForGetJob(job *model.Job) { }) } -// getFirstDDLJob gets the first DDL job form DDL queue. -func (w *worker) getFirstDDLJob(t *meta.Meta) (*model.Job, error) { - job, err := t.GetDDLJobByIdx(0) - injectFailPointForGetJob(job) - return job, errors.Trace(err) -} - // handleUpdateJobError handles the too large DDL job. func (w *worker) handleUpdateJobError(t *meta.Meta, job *model.Job, err error) error { if err == nil { @@ -471,7 +408,7 @@ func (w *worker) handleUpdateJobError(t *meta.Meta, job *model.Job, err error) e // updateDDLJob updates the DDL job information. // Every time we enter another state except final state, we must call this function. -func (w *worker) updateDDLJob(t *meta.Meta, job *model.Job, meetErr bool) error { +func (w *worker) updateDDLJob(job *model.Job, meetErr bool) error { failpoint.Inject("mockErrEntrySizeTooLarge", func(val failpoint.Value) { if val.(bool) { failpoint.Return(kv.ErrEntryTooLarge) @@ -482,13 +419,7 @@ func (w *worker) updateDDLJob(t *meta.Meta, job *model.Job, meetErr bool) error logutil.Logger(w.logCtx).Info("[ddl] meet something wrong before update DDL job, shouldn't update raw args", zap.String("job", job.String())) } - var err error - if w.concurrentDDL { - err = updateDDLJob2Table(w.sess, job, updateRawArgs) - } else { - err = t.UpdateDDLJob(0, job, updateRawArgs) - } - return errors.Trace(err) + return errors.Trace(updateDDLJob2Table(w.sess, job, updateRawArgs)) } // registerMDLInfo registers metadata lock info. @@ -631,11 +562,7 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { if err != nil { return errors.Trace(err) } - if w.concurrentDDL { - err = w.deleteDDLJob(job) - } else { - _, err = t.DeQueueDDLJob() - } + err = w.deleteDDLJob(job) if err != nil { return errors.Trace(err) } @@ -650,7 +577,7 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { } w.writeDDLSeqNum(job) w.removeJobCtx(job) - err = AddHistoryDDLJob(w.sess, t, job, updateRawArgs, w.concurrentDDL) + err = AddHistoryDDLJob(w.sess, t, job, updateRawArgs) return errors.Trace(err) } @@ -714,13 +641,6 @@ func isDependencyJobDone(t *meta.Meta, job *model.Job) (bool, error) { return true, nil } -func newMetaWithQueueTp(txn kv.Transaction, tp workerType) *meta.Meta { - if tp == addIdxWorker { - return meta.NewMeta(txn, meta.AddIndexJobListKey) - } - return meta.NewMeta(txn) -} - func (w *JobContext) setDDLLabelForTopSQL(job *model.Job) { if !topsqlstate.TopSQLEnabled() || job == nil { return @@ -797,7 +717,7 @@ func (w *worker) HandleDDLJobTable(d *ddlCtx, job *model.Job) (int64, error) { if err != nil { return 0, err } - if !variable.EnableConcurrentDDL.Load() || d.waiting.Load() { + if d.waiting.Load() { w.sess.rollback() return 0, nil } @@ -867,7 +787,7 @@ func (w *worker) HandleDDLJobTable(d *ddlCtx, job *model.Job) (int64, error) { d.unlockSchemaVersion(job.ID) return 0, err } - err = w.updateDDLJob(t, job, runJobErr != nil) + err = w.updateDDLJob(job, runJobErr != nil) if err = w.handleUpdateJobError(t, job, err); err != nil { w.sess.rollback() d.unlockSchemaVersion(job.ID) @@ -911,152 +831,6 @@ func (w *JobContext) ddlJobSourceType() string { return w.tp } -// handleDDLJobQueue handles DDL jobs in DDL Job queue. -func (w *worker) handleDDLJobQueue(d *ddlCtx) error { - once := true - waitDependencyJobCnt := 0 - for { - if isChanClosed(w.ctx.Done()) { - return nil - } - - var ( - job *model.Job - schemaVer int64 - runJobErr error - ) - waitTime := 2 * d.lease - ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL) - err := kv.RunInNewTxn(ctx, d.store, false, func(ctx context.Context, txn kv.Transaction) error { - d.runningJobs.Lock() - // We are not owner, return and retry checking later. - if !d.isOwner() || variable.EnableConcurrentDDL.Load() || d.waiting.Load() { - d.runningJobs.Unlock() - return nil - } - - var err error - t := newMetaWithQueueTp(txn, w.tp) - - // We become the owner. Get the first job and run it. - job, err = w.getFirstDDLJob(t) - if job == nil || err != nil { - d.runningJobs.Unlock() - return errors.Trace(err) - } - d.runningJobs.ids[job.ID] = struct{}{} - d.runningJobs.Unlock() - - defer d.deleteRunningDDLJobMap(job.ID) - - // only general ddls allowed to be executed when TiKV is disk full. - if w.tp == addIdxWorker && job.IsRunning() { - txn.SetDiskFullOpt(kvrpcpb.DiskFullOpt_NotAllowedOnFull) - } - - w.setDDLLabelForTopSQL(job) - w.setDDLSourceForDiagnosis(job) - jobContext := w.jobContext(job) - if tagger := w.getResourceGroupTaggerForTopSQL(job); tagger != nil { - txn.SetOption(kv.ResourceGroupTagger, tagger) - } - if isDone, err1 := isDependencyJobDone(t, job); err1 != nil || !isDone { - return errors.Trace(err1) - } - - if once { - err = waitSchemaSynced(d, job, waitTime) - if err == nil { - once = false - } - return err - } - - if job.IsDone() || job.IsRollbackDone() { - if !job.IsRollbackDone() { - job.State = model.JobStateSynced - } - err = w.finishDDLJob(t, job) - return errors.Trace(err) - } - - d.mu.RLock() - d.mu.hook.OnJobRunBefore(job) - d.mu.RUnlock() - - // set request source type to DDL type - txn.SetOption(kv.RequestSourceType, jobContext.ddlJobSourceType()) - // If running job meets error, we will save this error in job Error - // and retry later if the job is not cancelled. - schemaVer, runJobErr = w.runDDLJob(d, t, job) - if job.IsCancelled() { - txn.Reset() - err = w.finishDDLJob(t, job) - return errors.Trace(err) - } - if runJobErr != nil && !job.IsRollingback() && !job.IsRollbackDone() { - // If the running job meets an error - // and the job state is rolling back, it means that we have already handled this error. - // Some DDL jobs (such as adding indexes) may need to update the table info and the schema version, - // then shouldn't discard the KV modification. - // And the job state is rollback done, it means the job was already finished, also shouldn't discard too. - // Otherwise, we should discard the KV modification when running job. - txn.Reset() - // If error happens after updateSchemaVersion(), then the schemaVer is updated. - // Result in the retry duration is up to 2 * lease. - schemaVer = 0 - } - err = w.updateDDLJob(t, job, runJobErr != nil) - if err = w.handleUpdateJobError(t, job, err); err != nil { - return errors.Trace(err) - } - writeBinlog(d.binlogCli, txn, job) - return nil - }) - - if runJobErr != nil { - // wait a while to retry again. If we don't wait here, DDL will retry this job immediately, - // which may act like a deadlock. - logutil.Logger(w.logCtx).Info("[ddl] run DDL job failed, sleeps a while then retries it.", - zap.Duration("waitTime", GetWaitTimeWhenErrorOccurred()), zap.Error(runJobErr)) - time.Sleep(GetWaitTimeWhenErrorOccurred()) - } - if job != nil { - d.unlockSchemaVersion(job.ID) - } - - if err != nil { - w.unlockSeqNum(err) - return errors.Trace(err) - } else if job == nil { - // No job now, return and retry getting later. - return nil - } - w.unlockSeqNum(err) - w.waitDependencyJobFinished(job, &waitDependencyJobCnt) - - // Here means the job enters another state (delete only, write only, public, etc...) or is cancelled. - // If the job is done or still running or rolling back, we will wait 2 * lease time to guarantee other servers to update - // the newest schema. - waitSchemaChanged(context.Background(), d, waitTime, schemaVer, job) - - if RunInGoTest { - // d.mu.hook is initialed from domain / test callback, which will force the owner host update schema diff synchronously. - d.mu.RLock() - d.mu.hook.OnSchemaStateChanged(schemaVer) - d.mu.RUnlock() - } - - d.mu.RLock() - d.mu.hook.OnJobUpdated(job) - d.mu.RUnlock() - - if job.IsSynced() || job.IsCancelled() || job.IsRollbackDone() { - asyncNotify(d.ddlJobDoneCh) - } - } -} - func skipWriteBinlog(job *model.Job) bool { switch job.Type { // ActionUpdateTiFlashReplicaStatus is a TiDB internal DDL, diff --git a/ddl/ddl_workerpool_test.go b/ddl/ddl_workerpool_test.go index e9f324ce9dff8..d8768507b8102 100644 --- a/ddl/ddl_workerpool_test.go +++ b/ddl/ddl_workerpool_test.go @@ -26,7 +26,7 @@ import ( func TestDDLWorkerPool(t *testing.T) { f := func() func() (pools.Resource, error) { return func() (pools.Resource, error) { - wk := newWorker(nil, addIdxWorker, nil, nil, nil, true) + wk := newWorker(nil, addIdxWorker, nil, nil, nil) return wk, nil } } diff --git a/ddl/generated_column.go b/ddl/generated_column.go index 678d803edf521..6c5e99dab2cd6 100644 --- a/ddl/generated_column.go +++ b/ddl/generated_column.go @@ -122,13 +122,19 @@ func findPositionRelativeColumn(cols []*table.Column, pos *ast.ColumnPosition) ( // findDependedColumnNames returns a set of string, which indicates // the names of the columns that are depended by colDef. -func findDependedColumnNames(colDef *ast.ColumnDef) (generated bool, colsMap map[string]struct{}) { +func findDependedColumnNames(schemaName model.CIStr, tableName model.CIStr, colDef *ast.ColumnDef) (generated bool, colsMap map[string]struct{}, err error) { colsMap = make(map[string]struct{}) for _, option := range colDef.Options { if option.Tp == ast.ColumnOptionGenerated { generated = true colNames := FindColumnNamesInExpr(option.Expr) for _, depCol := range colNames { + if depCol.Schema.L != "" && schemaName.L != "" && depCol.Schema.L != schemaName.L { + return false, nil, dbterror.ErrWrongDBName.GenWithStackByArgs(depCol.Schema.O) + } + if depCol.Table.L != "" && tableName.L != "" && depCol.Table.L != tableName.L { + return false, nil, dbterror.ErrWrongTableName.GenWithStackByArgs(depCol.Table.O) + } colsMap[depCol.Name.L] = struct{}{} } break @@ -192,7 +198,7 @@ func (c *generatedColumnChecker) Leave(inNode ast.Node) (node ast.Node, ok bool) // 3. check if the modified expr contains non-deterministic functions // 4. check whether new column refers to any auto-increment columns. // 5. check if the new column is indexed or stored -func checkModifyGeneratedColumn(sctx sessionctx.Context, tbl table.Table, oldCol, newCol *table.Column, newColDef *ast.ColumnDef, pos *ast.ColumnPosition) error { +func checkModifyGeneratedColumn(sctx sessionctx.Context, schemaName model.CIStr, tbl table.Table, oldCol, newCol *table.Column, newColDef *ast.ColumnDef, pos *ast.ColumnPosition) error { // rule 1. oldColIsStored := !oldCol.IsGenerated() || oldCol.GeneratedStored newColIsStored := !newCol.IsGenerated() || newCol.GeneratedStored @@ -252,7 +258,10 @@ func checkModifyGeneratedColumn(sctx sessionctx.Context, tbl table.Table, oldCol } // rule 4. - _, dependColNames := findDependedColumnNames(newColDef) + _, dependColNames, err := findDependedColumnNames(schemaName, tbl.Meta().Name, newColDef) + if err != nil { + return errors.Trace(err) + } if !sctx.GetSessionVars().EnableAutoIncrementInGenerated { if err := checkAutoIncrementRef(newColDef.Name.Name.L, dependColNames, tbl.Meta()); err != nil { return errors.Trace(err) diff --git a/ddl/index.go b/ddl/index.go index 929a86b08321b..306723964a693 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -880,7 +880,7 @@ func doReorgWorkForCreateIndex(w *worker, d *ddlCtx, t *meta.Meta, job *model.Jo func runReorgJobAndHandleErr(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table, indexInfo *model.IndexInfo, mergingTmpIdx bool) (done bool, ver int64, err error) { elements := []*meta.Element{{ID: indexInfo.ID, TypeKey: meta.IndexElementKey}} - rh := newReorgHandler(t, w.sess, w.concurrentDDL) + rh := newReorgHandler(t, w.sess) dbInfo, err := t.GetDatabase(job.SchemaID) if err != nil { return false, ver, errors.Trace(err) diff --git a/ddl/job_table.go b/ddl/job_table.go index 117b3722eccde..06f745506145a 100644 --- a/ddl/job_table.go +++ b/ddl/job_table.go @@ -174,7 +174,7 @@ func (d *ddl) startDispatchLoop() { if isChanClosed(d.ctx.Done()) { return } - if !variable.EnableConcurrentDDL.Load() || !d.isOwner() || d.waiting.Load() { + if !d.isOwner() || d.waiting.Load() { d.once.Store(true) time.Sleep(time.Second) continue @@ -547,6 +547,19 @@ func addBackfillJobs(sess *session, tableName string, backfillJobs []*BackfillJo }) } +func runInTxn(se *session, f func(*session) error) (err error) { + err = se.begin() + if err != nil { + return err + } + err = f(se) + if err != nil { + se.rollback() + return + } + return errors.Trace(se.commit()) +} + // GetBackfillJobsForOneEle batch gets the backfill jobs in the tblName table that contains only one element. func GetBackfillJobsForOneEle(sess *session, batch int, excludedJobIDs []int64, lease time.Duration) ([]*BackfillJob, error) { eJobIDsBuilder := strings.Builder{} @@ -718,135 +731,3 @@ func updateBackfillJob(sess *session, tableName string, backfillJob *BackfillJob _, err = sess.execute(context.Background(), sql, label) return err } - -// MoveJobFromQueue2Table move existing DDLs in queue to table. -func (d *ddl) MoveJobFromQueue2Table(inBootstrap bool) error { - sess, err := d.sessPool.get() - if err != nil { - return err - } - defer d.sessPool.put(sess) - return runInTxn(newSession(sess), func(se *session) error { - txn, err := se.txn() - if err != nil { - return errors.Trace(err) - } - t := meta.NewMeta(txn) - isConcurrentDDL, err := t.IsConcurrentDDL() - if !inBootstrap && (isConcurrentDDL || err != nil) { - return errors.Trace(err) - } - systemDBID, err := t.GetSystemDBID() - if err != nil { - return errors.Trace(err) - } - for _, tp := range []workerType{addIdxWorker, generalWorker} { - t := newMetaWithQueueTp(txn, tp) - jobs, err := t.GetAllDDLJobsInQueue() - if err != nil { - return errors.Trace(err) - } - for _, job := range jobs { - // In bootstrap, we can ignore the internal DDL. - if inBootstrap && job.SchemaID == systemDBID { - continue - } - err = insertDDLJobs2Table(se, false, job) - if err != nil { - return errors.Trace(err) - } - if tp == generalWorker { - // General job do not have reorg info. - continue - } - element, start, end, pid, err := t.GetDDLReorgHandle(job) - if meta.ErrDDLReorgElementNotExist.Equal(err) { - continue - } - if err != nil { - return errors.Trace(err) - } - err = initDDLReorgHandle(se, job.ID, start, end, pid, element) - if err != nil { - return errors.Trace(err) - } - } - } - - if err = t.ClearALLDDLJob(); err != nil { - return errors.Trace(err) - } - if err = t.ClearAllDDLReorgHandle(); err != nil { - return errors.Trace(err) - } - return t.SetConcurrentDDL(true) - }) -} - -// MoveJobFromTable2Queue move existing DDLs in table to queue. -func (d *ddl) MoveJobFromTable2Queue() error { - sess, err := d.sessPool.get() - if err != nil { - return err - } - defer d.sessPool.put(sess) - return runInTxn(newSession(sess), func(se *session) error { - txn, err := se.txn() - if err != nil { - return errors.Trace(err) - } - t := meta.NewMeta(txn) - isConcurrentDDL, err := t.IsConcurrentDDL() - if !isConcurrentDDL || err != nil { - return errors.Trace(err) - } - jobs, err := getJobsBySQL(se, "tidb_ddl_job", "1 order by job_id") - if err != nil { - return errors.Trace(err) - } - - for _, job := range jobs { - jobListKey := meta.DefaultJobListKey - if job.MayNeedReorg() { - jobListKey = meta.AddIndexJobListKey - } - if err := t.EnQueueDDLJobNoUpdate(job, jobListKey); err != nil { - return errors.Trace(err) - } - } - - reorgHandle, err := se.execute(context.Background(), "select job_id, start_key, end_key, physical_id, ele_id, ele_type from mysql.tidb_ddl_reorg", "get_handle") - if err != nil { - return errors.Trace(err) - } - for _, row := range reorgHandle { - if err := t.UpdateDDLReorgHandle(row.GetInt64(0), row.GetBytes(1), row.GetBytes(2), row.GetInt64(3), &meta.Element{ID: row.GetInt64(4), TypeKey: row.GetBytes(5)}); err != nil { - return errors.Trace(err) - } - } - - // clean up these 2 tables. - _, err = se.execute(context.Background(), "delete from mysql.tidb_ddl_job", "delete_old_ddl") - if err != nil { - return errors.Trace(err) - } - _, err = se.execute(context.Background(), "delete from mysql.tidb_ddl_reorg", "delete_old_reorg") - if err != nil { - return errors.Trace(err) - } - return t.SetConcurrentDDL(false) - }) -} - -func runInTxn(se *session, f func(*session) error) (err error) { - err = se.begin() - if err != nil { - return err - } - err = f(se) - if err != nil { - se.rollback() - return - } - return errors.Trace(se.commit()) -} diff --git a/ddl/job_table_test.go b/ddl/job_table_test.go index ca30cf903107d..8948796e73243 100644 --- a/ddl/job_table_test.go +++ b/ddl/job_table_test.go @@ -26,7 +26,6 @@ import ( "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/sessiontxn" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tidb/types" @@ -40,9 +39,6 @@ import ( // This test checks the chosen job records to see if there are wrong scheduling, if job A and job B cannot run concurrently, // then the all the record of job A must before or after job B, no cross record between these 2 jobs should be in between. func TestDDLScheduling(t *testing.T) { - if !variable.EnableConcurrentDDL.Load() { - t.Skipf("test requires concurrent ddl") - } store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) diff --git a/ddl/multi_schema_change_test.go b/ddl/multi_schema_change_test.go index e211e9d51ca77..9f84ca6aeeed8 100644 --- a/ddl/multi_schema_change_test.go +++ b/ddl/multi_schema_change_test.go @@ -1263,7 +1263,7 @@ func (c *cancelOnceHook) OnJobUpdated(job *model.Job) { return } c.triggered = true - errs, err := ddl.CancelJobs(c.s, c.store, []int64{job.ID}) + errs, err := ddl.CancelJobs(c.s, []int64{job.ID}) if errs[0] != nil { c.cancelErr = errs[0] return diff --git a/ddl/partition.go b/ddl/partition.go index 2c95f389707f9..37b9f8bb2f5c7 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -1756,7 +1756,7 @@ func (w *worker) onDropTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) ( elements = append(elements, &meta.Element{ID: idxInfo.ID, TypeKey: meta.IndexElementKey}) } } - rh := newReorgHandler(t, w.sess, w.concurrentDDL) + rh := newReorgHandler(t, w.sess) reorgInfo, err := getReorgInfoFromPartitions(d.jobContext(job), d, rh, job, dbInfo, tbl, physicalTableIDs, elements) if err != nil || reorgInfo.first { diff --git a/ddl/reorg.go b/ddl/reorg.go index d7671031f64d1..a394d1682db82 100644 --- a/ddl/reorg.go +++ b/ddl/reorg.go @@ -34,7 +34,6 @@ import ( "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/statistics" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" @@ -275,8 +274,7 @@ func (w *worker) runReorgJob(rh *reorgHandler, reorgInfo *reorgInfo, tblInfo *mo // Update a reorgInfo's handle. // Since daemon-worker is triggered by timer to store the info half-way. // you should keep these infos is read-only (like job) / atomic (like doneKey & element) / concurrent safe. - err := rh.UpdateDDLReorgStartHandle(job, currentElement, doneKey) - + err := updateDDLReorgStartHandle(rh.s, job, currentElement, doneKey) logutil.BgLogger().Info("[ddl] run reorg job wait timeout", zap.Duration("wait time", waitTimeout), zap.ByteString("element type", currentElement.TypeKey), @@ -673,7 +671,7 @@ func getReorgInfo(ctx *JobContext, d *ddlCtx, rh *reorgHandler, job *model.Job, // We'll try to remove it in the next major TiDB version. if meta.ErrDDLReorgElementNotExist.Equal(err) { job.SnapshotVer = 0 - logutil.BgLogger().Warn("[ddl] get reorg info, the element does not exist", zap.String("job", job.String()), zap.Bool("enableConcurrentDDL", rh.enableConcurrentDDL)) + logutil.BgLogger().Warn("[ddl] get reorg info, the element does not exist", zap.String("job", job.String())) } return &info, errors.Trace(err) } @@ -772,8 +770,8 @@ func (r *reorgInfo) UpdateReorgMeta(startKey kv.Key, pool *sessionPool) (err err sess.rollback() return err } - rh := newReorgHandler(meta.NewMeta(txn), sess, variable.EnableConcurrentDDL.Load()) - err = rh.UpdateDDLReorgHandle(r.Job, startKey, r.EndKey, r.PhysicalTableID, r.currElement) + rh := newReorgHandler(meta.NewMeta(txn), sess) + err = updateDDLReorgHandle(rh.s, r.Job.ID, startKey, r.EndKey, r.PhysicalTableID, r.currElement) err1 := sess.commit() if err == nil { err = err1 @@ -785,63 +783,33 @@ func (r *reorgInfo) UpdateReorgMeta(startKey kv.Key, pool *sessionPool) (err err type reorgHandler struct { m *meta.Meta s *session - - enableConcurrentDDL bool } // NewReorgHandlerForTest creates a new reorgHandler, only used in test. func NewReorgHandlerForTest(t *meta.Meta, sess sessionctx.Context) *reorgHandler { - return newReorgHandler(t, newSession(sess), variable.EnableConcurrentDDL.Load()) + return newReorgHandler(t, newSession(sess)) } -func newReorgHandler(t *meta.Meta, sess *session, enableConcurrentDDL bool) *reorgHandler { - return &reorgHandler{m: t, s: sess, enableConcurrentDDL: enableConcurrentDDL} -} - -// UpdateDDLReorgStartHandle saves the job reorganization latest processed element and start handle for later resuming. -func (r *reorgHandler) UpdateDDLReorgStartHandle(job *model.Job, element *meta.Element, startKey kv.Key) error { - if r.enableConcurrentDDL { - return updateDDLReorgStartHandle(r.s, job, element, startKey) - } - return r.m.UpdateDDLReorgStartHandle(job, element, startKey) -} - -// UpdateDDLReorgHandle saves the job reorganization latest processed information for later resuming. -func (r *reorgHandler) UpdateDDLReorgHandle(job *model.Job, startKey, endKey kv.Key, physicalTableID int64, element *meta.Element) error { - if r.enableConcurrentDDL { - return updateDDLReorgHandle(r.s, job.ID, startKey, endKey, physicalTableID, element) - } - return r.m.UpdateDDLReorgHandle(job.ID, startKey, endKey, physicalTableID, element) +func newReorgHandler(t *meta.Meta, sess *session) *reorgHandler { + return &reorgHandler{m: t, s: sess} } // InitDDLReorgHandle initializes the job reorganization information. func (r *reorgHandler) InitDDLReorgHandle(job *model.Job, startKey, endKey kv.Key, physicalTableID int64, element *meta.Element) error { - if r.enableConcurrentDDL { - return initDDLReorgHandle(r.s, job.ID, startKey, endKey, physicalTableID, element) - } - return r.m.UpdateDDLReorgHandle(job.ID, startKey, endKey, physicalTableID, element) + return initDDLReorgHandle(r.s, job.ID, startKey, endKey, physicalTableID, element) } // RemoveReorgElementFailPoint removes the element of the reorganization information. func (r *reorgHandler) RemoveReorgElementFailPoint(job *model.Job) error { - if r.enableConcurrentDDL { - return removeReorgElement(r.s, job) - } - return r.m.RemoveReorgElement(job) + return removeReorgElement(r.s, job) } // RemoveDDLReorgHandle removes the job reorganization related handles. func (r *reorgHandler) RemoveDDLReorgHandle(job *model.Job, elements []*meta.Element) error { - if r.enableConcurrentDDL { - return removeDDLReorgHandle(r.s, job, elements) - } - return r.m.RemoveDDLReorgHandle(job, elements) + return removeDDLReorgHandle(r.s, job, elements) } // GetDDLReorgHandle gets the latest processed DDL reorganize position. func (r *reorgHandler) GetDDLReorgHandle(job *model.Job) (element *meta.Element, startKey, endKey kv.Key, physicalTableID int64, err error) { - if r.enableConcurrentDDL { - return getDDLReorgHandle(r.s, job) - } - return r.m.GetDDLReorgHandle(job) + return getDDLReorgHandle(r.s, job) } diff --git a/ddl/schema_test.go b/ddl/schema_test.go index 70206ed2f179f..3be4fb4e4d278 100644 --- a/ddl/schema_test.go +++ b/ddl/schema_test.go @@ -163,13 +163,13 @@ func testDropSchema(t *testing.T, ctx sessionctx.Context, d ddl.DDL, dbInfo *mod return job, ver } -func isDDLJobDone(test *testing.T, t *meta.Meta) bool { - job, err := t.GetDDLJobByIdx(0) - require.NoError(test, err) - if job == nil { +func isDDLJobDone(test *testing.T, t *meta.Meta, store kv.Storage) bool { + tk := testkit.NewTestKit(test, store) + rows := tk.MustQuery("select * from mysql.tidb_ddl_job").Rows() + + if len(rows) == 0 { return true } - time.Sleep(testLease) return false } @@ -185,7 +185,7 @@ func testCheckSchemaState(test *testing.T, store kv.Storage, dbInfo *model.DBInf require.NoError(test, err) if state == model.StateNone { - isDropped = isDDLJobDone(test, t) + isDropped = isDDLJobDone(test, t, store) if !isDropped { return nil } diff --git a/ddl/schematracker/checker.go b/ddl/schematracker/checker.go index b1533d0246fb1..6e38f6c8bdb79 100644 --- a/ddl/schematracker/checker.go +++ b/ddl/schematracker/checker.go @@ -541,16 +541,6 @@ func (d Checker) DoDDLJob(ctx sessionctx.Context, job *model.Job) error { return d.realDDL.DoDDLJob(ctx, job) } -// MoveJobFromQueue2Table implements the DDL interface. -func (d Checker) MoveJobFromQueue2Table(bool) error { - panic("implement me") -} - -// MoveJobFromTable2Queue implements the DDL interface. -func (d Checker) MoveJobFromTable2Queue() error { - panic("implement me") -} - // StorageDDLInjector wraps kv.Storage to inject checker to domain's DDL in bootstrap time. type StorageDDLInjector struct { kv.Storage diff --git a/ddl/schematracker/dm_tracker.go b/ddl/schematracker/dm_tracker.go index 75f8fa35b429d..5d3f693deaa0f 100644 --- a/ddl/schematracker/dm_tracker.go +++ b/ddl/schematracker/dm_tracker.go @@ -1256,13 +1256,3 @@ func (SchemaTracker) GetInfoSchemaWithInterceptor(ctx sessionctx.Context) infosc func (SchemaTracker) DoDDLJob(ctx sessionctx.Context, job *model.Job) error { return nil } - -// MoveJobFromQueue2Table implements the DDL interface, it's no-op in DM's case. -func (SchemaTracker) MoveJobFromQueue2Table(b bool) error { - panic("implement me") -} - -// MoveJobFromTable2Queue implements the DDL interface, it's no-op in DM's case. -func (SchemaTracker) MoveJobFromTable2Queue() error { - panic("implement me") -} diff --git a/ddl/stat_test.go b/ddl/stat_test.go index 556b9eb5dadc7..db8abc45be30c 100644 --- a/ddl/stat_test.go +++ b/ddl/stat_test.go @@ -25,14 +25,12 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/kv" - "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/sessiontxn" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/testkit" @@ -152,20 +150,13 @@ func TestGetDDLInfo(t *testing.T) { } func addDDLJobs(sess session.Session, txn kv.Transaction, job *model.Job) error { - if variable.EnableConcurrentDDL.Load() { - b, err := job.Encode(true) - if err != nil { - return err - } - _, err = sess.Execute(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), fmt.Sprintf("insert into mysql.tidb_ddl_job(job_id, reorg, schema_ids, table_ids, job_meta, type, processing) values (%d, %t, %s, %s, %s, %d, %t)", - job.ID, job.MayNeedReorg(), strconv.Quote(strconv.FormatInt(job.SchemaID, 10)), strconv.Quote(strconv.FormatInt(job.TableID, 10)), wrapKey2String(b), job.Type, false)) + b, err := job.Encode(true) + if err != nil { return err } - m := meta.NewMeta(txn) - if job.MayNeedReorg() { - return m.EnQueueDDLJob(job, meta.AddIndexJobListKey) - } - return m.EnQueueDDLJob(job) + _, err = sess.Execute(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), fmt.Sprintf("insert into mysql.tidb_ddl_job(job_id, reorg, schema_ids, table_ids, job_meta, type, processing) values (%d, %t, %s, %s, %s, %d, %t)", + job.ID, job.MayNeedReorg(), strconv.Quote(strconv.FormatInt(job.SchemaID, 10)), strconv.Quote(strconv.FormatInt(job.TableID, 10)), wrapKey2String(b), job.Type, false)) + return err } func wrapKey2String(key []byte) string { diff --git a/domain/BUILD.bazel b/domain/BUILD.bazel index 55f68e1ca20bd..eafaa6b4cd8aa 100644 --- a/domain/BUILD.bazel +++ b/domain/BUILD.bazel @@ -36,6 +36,7 @@ go_library( "//meta", "//metrics", "//owner", + "//parser", "//parser/ast", "//parser/model", "//parser/mysql", @@ -45,7 +46,6 @@ go_library( "//sessionctx/sessionstates", "//sessionctx/stmtctx", "//sessionctx/variable", - "//statistics", "//statistics/handle", "//telemetry", "//ttl/ttlworker", diff --git a/domain/plan_replayer.go b/domain/plan_replayer.go index fc54d30759057..54c109cc34dc3 100644 --- a/domain/plan_replayer.go +++ b/domain/plan_replayer.go @@ -29,13 +29,12 @@ import ( "github.com/pingcap/tidb/bindinfo" "github.com/pingcap/tidb/domain/infosync" "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/parser" "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/sessionctx/variable" - "github.com/pingcap/tidb/statistics" - "github.com/pingcap/tidb/statistics/handle" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/replayer" @@ -101,7 +100,7 @@ func (p *dumpFileGcChecker) gcDumpFilesByPath(path string, t time.Duration) { logutil.BgLogger().Error("[dumpFileGcChecker] parseTime failed", zap.Error(err), zap.String("filename", fileName)) continue } - isPlanReplayer := parseType(fileName) == "replayer" + isPlanReplayer := strings.Contains(fileName, "replayer") if !createTime.After(gcTime) { err := os.Remove(filepath.Join(path, f.Name())) if err != nil { @@ -410,7 +409,7 @@ func (w *planReplayerTaskDumpWorker) HandleTask(task *PlanReplayerDumpTask) (suc return true } - file, fileName, err := replayer.GeneratePlanReplayerFile(task.IsContinuesCapture) + file, fileName, err := replayer.GeneratePlanReplayerFile(task.IsCapture) if err != nil { logutil.BgLogger().Warn("[plan-replayer-capture] generate task file failed", zap.String("sqlDigest", taskKey.SQLDigest), @@ -421,29 +420,18 @@ func (w *planReplayerTaskDumpWorker) HandleTask(task *PlanReplayerDumpTask) (suc task.Zf = file task.FileName = fileName task.EncodedPlan, _ = task.EncodePlan(task.SessionVars.StmtCtx, false) - jsStats := make(map[int64]*handle.JSONTable) - is := GetDomain(w.sctx).InfoSchema() - if task.IsCapture && !task.IsContinuesCapture { - for tblID, stat := range task.TblStats { - tbl, ok := is.TableByID(tblID) - if !ok { - return false - } - schema, ok := is.SchemaByTable(tbl.Meta()) - if !ok { - return false - } - r, err := handle.GenJSONTableFromStats(schema.Name.String(), tbl.Meta(), stat.(*statistics.Table)) - if err != nil { - logutil.BgLogger().Warn("[plan-replayer-capture] generate task json stats failed", - zap.String("sqlDigest", taskKey.SQLDigest), - zap.String("planDigest", taskKey.PlanDigest), - zap.Error(err)) - return false - } - jsStats[tblID] = r + if task.InExecute && len(task.NormalizedSQL) > 0 { + p := parser.New() + stmts, _, err := p.ParseSQL(task.NormalizedSQL) + if err != nil { + logutil.BgLogger().Warn("[plan-replayer-capture] parse normalized sql failed", + zap.String("sql", task.NormalizedSQL), + zap.String("sqlDigest", taskKey.SQLDigest), + zap.String("planDigest", taskKey.PlanDigest), + zap.Error(err)) + return false } - task.JSONTblStats = jsStats + task.ExecStmts = stmts } err = DumpPlanReplayerInfo(w.ctx, w.sctx, task) if err != nil { @@ -538,15 +526,16 @@ type PlanReplayerDumpTask struct { replayer.PlanReplayerTaskKey // tmp variables stored during the query - EncodePlan func(*stmtctx.StatementContext, bool) (string, string) - TblStats map[int64]interface{} + EncodePlan func(*stmtctx.StatementContext, bool) (string, string) + TblStats map[int64]interface{} + InExecute bool + NormalizedSQL string // variables used to dump the plan StartTS uint64 SessionBindings []*bindinfo.BindRecord EncodedPlan string SessionVars *variable.SessionVars - JSONTblStats map[int64]*handle.JSONTable ExecStmts []ast.StmtNode Analyze bool diff --git a/domain/plan_replayer_dump.go b/domain/plan_replayer_dump.go index 0dd4945873e58..cad0898c81ef2 100644 --- a/domain/plan_replayer_dump.go +++ b/domain/plan_replayer_dump.go @@ -265,10 +265,10 @@ func DumpPlanReplayerInfo(ctx context.Context, sctx sessionctx.Context, return err } - // For continues capture, we don't dump stats - if !task.IsContinuesCapture { + // For capture task, we don't dump stats + if !task.IsCapture { // Dump stats - if err = dumpStats(zw, pairs, task.JSONTblStats, do); err != nil { + if err = dumpStats(zw, pairs, do); err != nil { return err } } @@ -415,12 +415,12 @@ func dumpSchemaMeta(zw *zip.Writer, tables map[tableNamePair]struct{}) error { return nil } -func dumpStats(zw *zip.Writer, pairs map[tableNamePair]struct{}, tblJSONStats map[int64]*handle.JSONTable, do *Domain) error { +func dumpStats(zw *zip.Writer, pairs map[tableNamePair]struct{}, do *Domain) error { for pair := range pairs { if pair.IsView { continue } - jsonTbl, err := getStatsForTable(do, tblJSONStats, pair) + jsonTbl, err := getStatsForTable(do, pair) if err != nil { return err } @@ -653,19 +653,14 @@ func extractTableNames(ctx context.Context, sctx sessionctx.Context, return r, nil } -func getStatsForTable(do *Domain, tblJSONStats map[int64]*handle.JSONTable, pair tableNamePair) (*handle.JSONTable, error) { +func getStatsForTable(do *Domain, pair tableNamePair) (*handle.JSONTable, error) { is := do.InfoSchema() h := do.StatsHandle() tbl, err := is.TableByName(model.NewCIStr(pair.DBName), model.NewCIStr(pair.TableName)) if err != nil { return nil, err } - js, ok := tblJSONStats[tbl.Meta().ID] - if ok && js != nil { - return js, nil - } - js, err = h.DumpStatsToJSON(pair.DBName, tbl.Meta(), nil, true) - return js, err + return h.DumpStatsToJSON(pair.DBName, tbl.Meta(), nil, true) } func getShowCreateTable(pair tableNamePair, zw *zip.Writer, ctx sessionctx.Context) error { diff --git a/executor/adapter.go b/executor/adapter.go index c087a50e5f5f0..a98bac9186b69 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/log" + "github.com/pingcap/tidb/bindinfo" "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl/placement" "github.com/pingcap/tidb/domain" @@ -58,6 +59,7 @@ import ( "github.com/pingcap/tidb/util/mathutil" "github.com/pingcap/tidb/util/memory" "github.com/pingcap/tidb/util/plancodec" + "github.com/pingcap/tidb/util/replayer" "github.com/pingcap/tidb/util/sqlexec" "github.com/pingcap/tidb/util/stmtsummary" "github.com/pingcap/tidb/util/stringutil" @@ -1365,6 +1367,18 @@ func (a *ExecStmt) observePhaseDurations(internal bool, commitDetails *util.Comm // 4. update the `PrevStmt` in session variable. // 5. reset `DurationParse` in session variable. func (a *ExecStmt) FinishExecuteStmt(txnTS uint64, err error, hasMoreResults bool) { + se := a.Ctx + if !se.GetSessionVars().InRestrictedSQL && se.GetSessionVars().IsPlanReplayerCaptureEnabled() { + stmtNode := a.GetStmtNode() + if se.GetSessionVars().EnablePlanReplayedContinuesCapture { + if checkPlanReplayerContinuesCaptureValidStmt(stmtNode) { + checkPlanReplayerContinuesCapture(se, stmtNode, txnTS) + } + } else { + checkPlanReplayerCaptureTask(se, stmtNode, txnTS) + } + } + sessVars := a.Ctx.GetSessionVars() execDetail := sessVars.StmtCtx.GetExecDetails() // Attach commit/lockKeys runtime stats to executor runtime stats. @@ -1953,3 +1967,86 @@ func convertStatusIntoString(sctx sessionctx.Context, statsLoadStatus map[model. } return r } + +// only allow select/delete/update/insert/execute stmt captured by continues capture +func checkPlanReplayerContinuesCaptureValidStmt(stmtNode ast.StmtNode) bool { + switch stmtNode.(type) { + case *ast.SelectStmt, *ast.DeleteStmt, *ast.UpdateStmt, *ast.InsertStmt, *ast.ExecuteStmt: + return true + default: + return false + } +} + +func checkPlanReplayerCaptureTask(sctx sessionctx.Context, stmtNode ast.StmtNode, startTS uint64) { + dom := domain.GetDomain(sctx) + if dom == nil { + return + } + handle := dom.GetPlanReplayerHandle() + if handle == nil { + return + } + tasks := handle.GetTasks() + _, sqlDigest := sctx.GetSessionVars().StmtCtx.SQLDigest() + _, planDigest := sctx.GetSessionVars().StmtCtx.GetPlanDigest() + key := replayer.PlanReplayerTaskKey{ + SQLDigest: sqlDigest.String(), + PlanDigest: planDigest.String(), + } + for _, task := range tasks { + if task.SQLDigest == sqlDigest.String() { + if task.PlanDigest == "*" || task.PlanDigest == planDigest.String() { + sendPlanReplayerDumpTask(key, sctx, stmtNode, startTS, false) + return + } + } + } +} + +func checkPlanReplayerContinuesCapture(sctx sessionctx.Context, stmtNode ast.StmtNode, startTS uint64) { + dom := domain.GetDomain(sctx) + if dom == nil { + return + } + handle := dom.GetPlanReplayerHandle() + if handle == nil { + return + } + _, sqlDigest := sctx.GetSessionVars().StmtCtx.SQLDigest() + _, planDigest := sctx.GetSessionVars().StmtCtx.GetPlanDigest() + key := replayer.PlanReplayerTaskKey{ + SQLDigest: sqlDigest.String(), + PlanDigest: planDigest.String(), + } + existed := sctx.GetSessionVars().CheckPlanReplayerFinishedTaskKey(key) + if existed { + return + } + sendPlanReplayerDumpTask(key, sctx, stmtNode, startTS, true) + sctx.GetSessionVars().AddPlanReplayerFinishedTaskKey(key) +} + +func sendPlanReplayerDumpTask(key replayer.PlanReplayerTaskKey, sctx sessionctx.Context, stmtNode ast.StmtNode, + startTS uint64, isContinuesCapture bool) { + stmtCtx := sctx.GetSessionVars().StmtCtx + handle := sctx.Value(bindinfo.SessionBindInfoKeyType).(*bindinfo.SessionHandle) + dumpTask := &domain.PlanReplayerDumpTask{ + PlanReplayerTaskKey: key, + StartTS: startTS, + EncodePlan: GetEncodedPlan, + TblStats: stmtCtx.TableStats, + SessionBindings: handle.GetAllBindRecord(), + SessionVars: sctx.GetSessionVars(), + ExecStmts: []ast.StmtNode{stmtNode}, + Analyze: false, + IsCapture: true, + IsContinuesCapture: isContinuesCapture, + } + if _, ok := stmtNode.(*ast.ExecuteStmt); ok { + nsql, _ := sctx.GetSessionVars().StmtCtx.SQLDigest() + dumpTask.InExecute = true + dumpTask.NormalizedSQL = nsql + } + domain.GetDomain(sctx).GetPlanReplayerHandle().SendTask(dumpTask) +} diff --git a/executor/builder.go b/executor/builder.go index f771c706bfa9b..c014893a0c86c 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -1724,7 +1724,7 @@ func (b *executorBuilder) buildTableDual(v *plannercore.PhysicalTableDual) Execu // `getSnapshotTS` returns for-update-ts if in insert/update/delete/lock statement otherwise the isolation read ts // Please notice that in RC isolation, the above two ts are the same -func (b *executorBuilder) getSnapshotTS() (uint64, error) { +func (b *executorBuilder) getSnapshotTS() (ts uint64, err error) { if b.forDataReaderBuilder { return b.dataReaderTS, nil } diff --git a/executor/compiler.go b/executor/compiler.go index 821561899f4e7..9f089eed9bae0 100644 --- a/executor/compiler.go +++ b/executor/compiler.go @@ -21,9 +21,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/pingcap/errors" "github.com/pingcap/failpoint" - "github.com/pingcap/tidb/bindinfo" "github.com/pingcap/tidb/config" - "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/mysql" @@ -34,7 +32,6 @@ import ( "github.com/pingcap/tidb/sessiontxn/staleread" "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/memory" - "github.com/pingcap/tidb/util/replayer" "go.uber.org/zap" ) @@ -157,111 +154,12 @@ func (c *Compiler) Compile(ctx context.Context, stmtNode ast.StmtNode) (_ *ExecS } } } - if err = sessiontxn.OptimizeWithPlanAndThenWarmUp(c.Ctx, stmt.Plan); err != nil { return nil, err } - - if c.Ctx.GetSessionVars().IsPlanReplayerCaptureEnabled() && !c.Ctx.GetSessionVars().InRestrictedSQL { - startTS, err := sessiontxn.GetTxnManager(c.Ctx).GetStmtReadTS() - if err != nil { - return nil, err - } - if c.Ctx.GetSessionVars().EnablePlanReplayedContinuesCapture { - checkPlanReplayerContinuesCapture(c.Ctx, stmtNode, startTS) - } else { - checkPlanReplayerCaptureTask(c.Ctx, stmtNode, startTS) - } - } return stmt, nil } -func checkPlanReplayerCaptureTask(sctx sessionctx.Context, stmtNode ast.StmtNode, startTS uint64) { - dom := domain.GetDomain(sctx) - if dom == nil { - return - } - handle := dom.GetPlanReplayerHandle() - if handle == nil { - return - } - captured := false - tasks := handle.GetTasks() - _, sqlDigest := sctx.GetSessionVars().StmtCtx.SQLDigest() - _, planDigest := getPlanDigest(sctx.GetSessionVars().StmtCtx) - defer func() { - logutil.BgLogger().Debug("[plan-replayer-capture] check capture task", - zap.String("sql-digest", sqlDigest.String()), - zap.String("plan-digest", planDigest.String()), - zap.Int("tasks", len(tasks)), - zap.Bool("captured", captured)) - }() - key := replayer.PlanReplayerTaskKey{ - SQLDigest: sqlDigest.String(), - PlanDigest: planDigest.String(), - } - for _, task := range tasks { - if task.SQLDigest == sqlDigest.String() { - if task.PlanDigest == "*" || task.PlanDigest == planDigest.String() { - captured = sendPlanReplayerDumpTask(key, sctx, stmtNode, startTS, false) - return - } - } - } -} - -func checkPlanReplayerContinuesCapture(sctx sessionctx.Context, stmtNode ast.StmtNode, startTS uint64) { - dom := domain.GetDomain(sctx) - if dom == nil { - return - } - handle := dom.GetPlanReplayerHandle() - if handle == nil { - return - } - _, sqlDigest := sctx.GetSessionVars().StmtCtx.SQLDigest() - _, planDigest := getPlanDigest(sctx.GetSessionVars().StmtCtx) - key := replayer.PlanReplayerTaskKey{ - SQLDigest: sqlDigest.String(), - PlanDigest: planDigest.String(), - } - captured := false - defer func() { - logutil.BgLogger().Debug("[plan-replayer-capture] check continues capture task", - zap.String("sql-digest", sqlDigest.String()), - zap.String("plan-digest", planDigest.String()), - zap.Bool("captured", captured)) - }() - - existed := sctx.GetSessionVars().CheckPlanReplayerFinishedTaskKey(key) - if existed { - return - } - captured = sendPlanReplayerDumpTask(key, sctx, stmtNode, startTS, true) - if captured { - sctx.GetSessionVars().AddPlanReplayerFinishedTaskKey(key) - } -} - -func sendPlanReplayerDumpTask(key replayer.PlanReplayerTaskKey, sctx sessionctx.Context, stmtNode ast.StmtNode, - startTS uint64, isContinuesCapture bool) bool { - stmtCtx := sctx.GetSessionVars().StmtCtx - handle := sctx.Value(bindinfo.SessionBindInfoKeyType).(*bindinfo.SessionHandle) - dumpTask := &domain.PlanReplayerDumpTask{ - PlanReplayerTaskKey: key, - StartTS: startTS, - EncodePlan: GetEncodedPlan, - TblStats: stmtCtx.TableStats, - SessionBindings: handle.GetAllBindRecord(), - SessionVars: sctx.GetSessionVars(), - ExecStmts: []ast.StmtNode{stmtNode}, - Analyze: false, - IsCapture: true, - IsContinuesCapture: isContinuesCapture, - } - return domain.GetDomain(sctx).GetPlanReplayerHandle().SendTask(dumpTask) -} - // needLowerPriority checks whether it's needed to lower the execution priority // of a query. // If the estimated output row count of any operator in the physical plan tree diff --git a/executor/executor.go b/executor/executor.go index 9f95e63aaed20..603996ad7764f 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -353,7 +353,7 @@ func (e *CancelDDLJobsExec) Open(ctx context.Context) error { if err != nil { return err } - e.errs, err = ddl.CancelJobs(newSess, e.ctx.GetStore(), e.jobIDs) + e.errs, err = ddl.CancelJobs(newSess, e.jobIDs) e.releaseSysSession(kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL), newSess) return err } diff --git a/executor/executor_test.go b/executor/executor_test.go index 122fbdbe7dd2f..858a4cc9372f9 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -178,12 +178,26 @@ func TestPlanReplayer(t *testing.T) { } func TestPlanReplayerCapture(t *testing.T) { - store := testkit.CreateMockStore(t) + store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") tk.MustExec("plan replayer capture '123' '123';") tk.MustQuery("select sql_digest, plan_digest from mysql.plan_replayer_task;").Check(testkit.Rows("123 123")) tk.MustGetErrMsg("plan replayer capture '123' '123';", "plan replayer capture task already exists") + tk.MustExec("delete from mysql.plan_replayer_task") + tk.MustExec("create table t(id int)") + tk.MustExec("prepare stmt from 'update t set id = ? where id = ? + 1';") + tk.MustExec("SET @number = 5;") + tk.MustExec("execute stmt using @number,@number") + _, sqlDigest := tk.Session().GetSessionVars().StmtCtx.SQLDigest() + _, planDigest := tk.Session().GetSessionVars().StmtCtx.GetPlanDigest() + tk.MustExec("SET @@tidb_enable_plan_replayer_capture = ON;") + tk.MustExec(fmt.Sprintf("plan replayer capture '%v' '%v'", sqlDigest.String(), planDigest.String())) + err := dom.GetPlanReplayerHandle().CollectPlanReplayerTask() + require.NoError(t, err) + tk.MustExec("execute stmt using @number,@number") + task := dom.GetPlanReplayerHandle().DrainTask() + require.NotNil(t, task) } func TestPlanReplayerContinuesCapture(t *testing.T) { diff --git a/executor/plan_replayer.go b/executor/plan_replayer.go index fae6273b3bd5e..ff102e20820b2 100644 --- a/executor/plan_replayer.go +++ b/executor/plan_replayer.go @@ -119,6 +119,11 @@ func (e *PlanReplayerExec) registerCaptureTask(ctx context.Context) error { zap.Error(err)) return err } + err = domain.GetDomain(e.ctx).GetPlanReplayerHandle().CollectPlanReplayerTask() + if err != nil { + logutil.BgLogger().Warn("collect task failed", zap.Error(err)) + } + logutil.BgLogger().Info("collect plan replayer task success") e.endFlag = true return nil } diff --git a/expression/integration_serial_test.go b/expression/integration_serial_test.go index 77574b4e309a2..b70b7be4a5070 100644 --- a/expression/integration_serial_test.go +++ b/expression/integration_serial_test.go @@ -3762,16 +3762,6 @@ func TestSetVariables(t *testing.T) { _, err = tk.Exec("set @@global.max_prepared_stmt_count='';") require.Error(t, err) require.Error(t, err, variable.ErrWrongTypeForVar.GenWithStackByArgs("max_prepared_stmt_count").Error()) - - tk.MustExec("set @@global.tidb_enable_concurrent_ddl=1") - tk.MustQuery("select @@global.tidb_enable_concurrent_ddl").Check(testkit.Rows("1")) - require.True(t, variable.EnableConcurrentDDL.Load()) - tk.MustExec("set @@global.tidb_enable_metadata_lock=0") - tk.MustExec("set @@global.tidb_enable_concurrent_ddl=0") - tk.MustQuery("select @@global.tidb_enable_concurrent_ddl").Check(testkit.Rows("0")) - require.False(t, variable.EnableConcurrentDDL.Load()) - testkit.NewTestKit(t, store).MustQuery("select @@global.tidb_enable_concurrent_ddl").Check(testkit.Rows("0")) - tk.MustExec("set @@global.tidb_enable_concurrent_ddl=1") } func TestPreparePlanCache(t *testing.T) { diff --git a/infoschema/cluster_tables_test.go b/infoschema/cluster_tables_test.go index 73488028a26ec..89244bf984631 100644 --- a/infoschema/cluster_tables_test.go +++ b/infoschema/cluster_tables_test.go @@ -41,7 +41,6 @@ import ( "github.com/pingcap/tidb/parser/auth" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/server" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/store/helper" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/mockstore/mockstorage" @@ -817,10 +816,6 @@ func (s *clusterTablesSuite) newTestKitWithRoot(t *testing.T) *testkit.TestKit { } func TestMDLView(t *testing.T) { - if !variable.EnableConcurrentDDL.Load() { - t.Skipf("test requires concurrent ddl") - } - // setup suite s := new(clusterTablesSuite) s.store, s.dom = testkit.CreateMockStoreAndDomain(t) diff --git a/infoschema/tables_test.go b/infoschema/tables_test.go index d4d4c4fa588f7..574529a822b13 100644 --- a/infoschema/tables_test.go +++ b/infoschema/tables_test.go @@ -1670,10 +1670,6 @@ func TestVariablesInfo(t *testing.T) { tk := testkit.NewTestKit(t, store) - if !variable.EnableConcurrentDDL.Load() { - t.Skip("skip test when concurrent DDL is disabled") - } - tk.MustExec("use information_schema") tk.MustExec("SET GLOBAL innodb_compression_level = 8;") diff --git a/meta/BUILD.bazel b/meta/BUILD.bazel index 791662be8c215..c6c796a9771c1 100644 --- a/meta/BUILD.bazel +++ b/meta/BUILD.bazel @@ -16,10 +16,8 @@ go_library( "//parser/mysql", "//structure", "//util/dbterror", - "//util/logutil", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/kvrpcpb", - "@org_uber_go_zap//:zap", ], ) @@ -33,12 +31,10 @@ go_test( embed = [":meta"], flaky = True, deps = [ - "//ddl", "//kv", "//parser/model", "//store/mockstore", "//testkit/testsetup", - "//testkit/testutil", "//util", "@com_github_pingcap_errors//:errors", "@com_github_stretchr_testify//require", diff --git a/meta/meta.go b/meta/meta.go index 9f262be8b464d..97f6756a582b2 100644 --- a/meta/meta.go +++ b/meta/meta.go @@ -19,7 +19,6 @@ import ( "encoding/binary" "encoding/json" "fmt" - "math" "strconv" "strings" "sync" @@ -34,8 +33,6 @@ import ( "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/structure" "github.com/pingcap/tidb/util/dbterror" - "github.com/pingcap/tidb/util/logutil" - "go.uber.org/zap" ) var ( @@ -77,7 +74,6 @@ var ( mPolicyGlobalID = []byte("PolicyGlobalID") mPolicyMagicByte = CurrentMagicByteVer mDDLTableVersion = []byte("DDLTableVersion") - mConcurrentDDL = []byte("concurrentDDL") mMetaDataLock = []byte("metadataLock") ) @@ -129,17 +125,13 @@ type Meta struct { // NewMeta creates a Meta in transaction txn. // If the current Meta needs to handle a job, jobListKey is the type of the job's list. -func NewMeta(txn kv.Transaction, jobListKeys ...JobListKeyType) *Meta { +func NewMeta(txn kv.Transaction) *Meta { txn.SetOption(kv.Priority, kv.PriorityHigh) txn.SetDiskFullOpt(kvrpcpb.DiskFullOpt_AllowedOnAlmostFull) t := structure.NewStructure(txn, txn, mMetaPrefix) - listKey := DefaultJobListKey - if len(jobListKeys) != 0 { - listKey = jobListKeys[0] - } return &Meta{txn: t, StartTS: txn.StartTS(), - jobListKey: listKey, + jobListKey: DefaultJobListKey, } } @@ -622,27 +614,6 @@ func (m *Meta) CheckMDLTableExists() (bool, error) { return bytes.Equal(v, []byte("2")), nil } -// SetConcurrentDDL set the concurrent DDL flag. -func (m *Meta) SetConcurrentDDL(b bool) error { - var data []byte - if b { - data = []byte("1") - } else { - data = []byte("0") - } - return errors.Trace(m.txn.Set(mConcurrentDDL, data)) -} - -// IsConcurrentDDL returns true if the concurrent DDL flag is set. -func (m *Meta) IsConcurrentDDL() (bool, error) { - val, err := m.txn.Get(mConcurrentDDL) - if err != nil { - return false, errors.Trace(err) - } - - return len(val) == 0 || bytes.Equal(val, []byte("1")), nil -} - // SetMetadataLock sets the metadata lock. func (m *Meta) SetMetadataLock(b bool) error { var data []byte @@ -987,12 +958,8 @@ var ( mDDLJobListKey = []byte("DDLJobList") mDDLJobAddIdxList = []byte("DDLJobAddIdxList") mDDLJobHistoryKey = []byte("DDLJobHistory") - mDDLJobReorgKey = []byte("DDLJobReorg") ) -// JobListKeyType is a key type of the DDL job queue. -type JobListKeyType []byte - var ( // DefaultJobListKey keeps all actions of DDL jobs except "add index". DefaultJobListKey JobListKeyType = mDDLJobListKey @@ -1018,31 +985,8 @@ func (m *Meta) EnQueueDDLJob(job *model.Job, jobListKeys ...JobListKeyType) erro return m.enQueueDDLJob(listKey, job, true) } -// EnQueueDDLJobNoUpdate adds a DDL job to the list without update raw args. -func (m *Meta) EnQueueDDLJobNoUpdate(job *model.Job, jobListKeys ...JobListKeyType) error { - listKey := m.jobListKey - if len(jobListKeys) != 0 { - listKey = jobListKeys[0] - } - - return m.enQueueDDLJob(listKey, job, false) -} - -func (m *Meta) deQueueDDLJob(key []byte) (*model.Job, error) { - value, err := m.txn.LPop(key) - if err != nil || value == nil { - return nil, errors.Trace(err) - } - - job := &model.Job{} - err = job.Decode(value) - return job, errors.Trace(err) -} - -// DeQueueDDLJob pops a DDL job from the list. -func (m *Meta) DeQueueDDLJob() (*model.Job, error) { - return m.deQueueDDLJob(m.jobListKey) -} +// JobListKeyType is a key type of the DDL job queue. +type JobListKeyType []byte func (m *Meta) getDDLJob(key []byte, index int64) (*model.Job, error) { value, err := m.txn.LIndex(key, index) @@ -1063,61 +1007,6 @@ func (m *Meta) getDDLJob(key []byte, index int64) (*model.Job, error) { return job, errors.Trace(err) } -// GetDDLJobByIdx returns the corresponding DDL job by the index. -// The length of jobListKeys can only be 1 or 0. -// If its length is 1, we need to replace m.jobListKey with jobListKeys[0]. -// Otherwise, we use m.jobListKey directly. -func (m *Meta) GetDDLJobByIdx(index int64, jobListKeys ...JobListKeyType) (*model.Job, error) { - listKey := m.jobListKey - if len(jobListKeys) != 0 { - listKey = jobListKeys[0] - } - - startTime := time.Now() - job, err := m.getDDLJob(listKey, index) - metrics.MetaHistogram.WithLabelValues(metrics.GetDDLJobByIdx, metrics.RetLabel(err)).Observe(time.Since(startTime).Seconds()) - return job, errors.Trace(err) -} - -// updateDDLJob updates the DDL job with index and key. -// updateRawArgs is used to determine whether to update the raw args when encode the job. -func (m *Meta) updateDDLJob(index int64, job *model.Job, key []byte, updateRawArgs bool) error { - b, err := job.Encode(updateRawArgs) - if err == nil { - err = m.txn.LSet(key, index, b) - } - return errors.Trace(err) -} - -// UpdateDDLJob updates the DDL job with index. -// updateRawArgs is used to determine whether to update the raw args when encode the job. -// The length of jobListKeys can only be 1 or 0. -// If its length is 1, we need to replace m.jobListKey with jobListKeys[0]. -// Otherwise, we use m.jobListKey directly. -func (m *Meta) UpdateDDLJob(index int64, job *model.Job, updateRawArgs bool, jobListKeys ...JobListKeyType) error { - listKey := m.jobListKey - if len(jobListKeys) != 0 { - listKey = jobListKeys[0] - } - - startTime := time.Now() - err := m.updateDDLJob(index, job, listKey, updateRawArgs) - metrics.MetaHistogram.WithLabelValues(metrics.UpdateDDLJob, metrics.RetLabel(err)).Observe(time.Since(startTime).Seconds()) - return errors.Trace(err) -} - -// DDLJobQueueLen returns the DDL job queue length. -// The length of jobListKeys can only be 1 or 0. -// If its length is 1, we need to replace m.jobListKey with jobListKeys[0]. -// Otherwise, we use m.jobListKey directly. -func (m *Meta) DDLJobQueueLen(jobListKeys ...JobListKeyType) (int64, error) { - listKey := m.jobListKey - if len(jobListKeys) != 0 { - listKey = jobListKeys[0] - } - return m.txn.LLen(listKey) -} - // GetAllDDLJobsInQueue gets all DDL Jobs in the current queue. // The length of jobListKeys can only be 1 or 0. // If its length is 1, we need to replace m.jobListKey with jobListKeys[0]. @@ -1152,45 +1041,6 @@ func (*Meta) jobIDKey(id int64) []byte { return b } -func (m *Meta) reorgJobCurrentElement(id int64) []byte { - b := make([]byte, 0, 12) - b = append(b, m.jobIDKey(id)...) - b = append(b, "_ele"...) - return b -} - -func (m *Meta) reorgJobStartHandle(id int64, element *Element) []byte { - b := make([]byte, 0, 16+len(element.TypeKey)) - b = append(b, m.jobIDKey(id)...) - b = append(b, element.TypeKey...) - eID := make([]byte, 8) - binary.BigEndian.PutUint64(eID, uint64(element.ID)) - b = append(b, eID...) - return b -} - -func (*Meta) reorgJobEndHandle(id int64, element *Element) []byte { - b := make([]byte, 8, 25) - binary.BigEndian.PutUint64(b, uint64(id)) - b = append(b, element.TypeKey...) - eID := make([]byte, 8) - binary.BigEndian.PutUint64(eID, uint64(element.ID)) - b = append(b, eID...) - b = append(b, "_end"...) - return b -} - -func (*Meta) reorgJobPhysicalTableID(id int64, element *Element) []byte { - b := make([]byte, 8, 25) - binary.BigEndian.PutUint64(b, uint64(id)) - b = append(b, element.TypeKey...) - eID := make([]byte, 8) - binary.BigEndian.PutUint64(eID, uint64(element.ID)) - b = append(b, eID...) - b = append(b, "_pid"...) - return b -} - func (m *Meta) addHistoryDDLJob(key []byte, job *model.Job, updateRawArgs bool) error { b, err := job.Encode(updateRawArgs) if err == nil { @@ -1352,160 +1202,6 @@ func DecodeElement(b []byte) (*Element, error) { return &Element{ID: int64(id), TypeKey: tp}, nil } -// UpdateDDLReorgStartHandle saves the job reorganization latest processed element and start handle for later resuming. -func (m *Meta) UpdateDDLReorgStartHandle(job *model.Job, element *Element, startKey kv.Key) error { - err := m.txn.HSet(mDDLJobReorgKey, m.reorgJobCurrentElement(job.ID), element.EncodeElement()) - if err != nil { - return errors.Trace(err) - } - if startKey != nil { - err = m.txn.HSet(mDDLJobReorgKey, m.reorgJobStartHandle(job.ID, element), startKey) - if err != nil { - return errors.Trace(err) - } - } - return nil -} - -// UpdateDDLReorgHandle saves the job reorganization latest processed information for later resuming. -func (m *Meta) UpdateDDLReorgHandle(jobID int64, startKey, endKey kv.Key, physicalTableID int64, element *Element) error { - err := m.txn.HSet(mDDLJobReorgKey, m.reorgJobCurrentElement(jobID), element.EncodeElement()) - if err != nil { - return errors.Trace(err) - } - if startKey != nil { - err = m.txn.HSet(mDDLJobReorgKey, m.reorgJobStartHandle(jobID, element), startKey) - if err != nil { - return errors.Trace(err) - } - } - if endKey != nil { - err = m.txn.HSet(mDDLJobReorgKey, m.reorgJobEndHandle(jobID, element), endKey) - if err != nil { - return errors.Trace(err) - } - } - err = m.txn.HSet(mDDLJobReorgKey, m.reorgJobPhysicalTableID(jobID, element), []byte(strconv.FormatInt(physicalTableID, 10))) - return errors.Trace(err) -} - -// ClearAllDDLReorgHandle clears all reorganization related handles. -func (m *Meta) ClearAllDDLReorgHandle() error { - return m.txn.HClear(mDDLJobReorgKey) -} - -// ClearALLDDLJob clears all DDL jobs. -func (m *Meta) ClearALLDDLJob() error { - if err := m.txn.LClear(mDDLJobAddIdxList); err != nil { - return errors.Trace(err) - } - if err := m.txn.LClear(mDDLJobListKey); err != nil { - return errors.Trace(err) - } - return nil -} - -// ClearAllHistoryJob clears all history jobs. **IT IS VERY DANGEROUS** -func (m *Meta) ClearAllHistoryJob() error { - if err := m.txn.HClear(mDDLJobHistoryKey); err != nil { - return errors.Trace(err) - } - return nil -} - -// RemoveReorgElement removes the element of the reorganization information. -func (m *Meta) RemoveReorgElement(job *model.Job) error { - err := m.txn.HDel(mDDLJobReorgKey, m.reorgJobCurrentElement(job.ID)) - if err != nil { - return errors.Trace(err) - } - return nil -} - -// RemoveDDLReorgHandle removes the job reorganization related handles. -func (m *Meta) RemoveDDLReorgHandle(job *model.Job, elements []*Element) error { - if len(elements) == 0 { - return nil - } - - err := m.txn.HDel(mDDLJobReorgKey, m.reorgJobCurrentElement(job.ID)) - if err != nil { - return errors.Trace(err) - } - - for _, element := range elements { - err = m.txn.HDel(mDDLJobReorgKey, m.reorgJobStartHandle(job.ID, element)) - if err != nil { - return errors.Trace(err) - } - if err = m.txn.HDel(mDDLJobReorgKey, m.reorgJobEndHandle(job.ID, element)); err != nil { - logutil.BgLogger().Warn("remove DDL reorg end handle", zap.Error(err)) - } - if err = m.txn.HDel(mDDLJobReorgKey, m.reorgJobPhysicalTableID(job.ID, element)); err != nil { - logutil.BgLogger().Warn("remove DDL reorg physical ID", zap.Error(err)) - } - } - return nil -} - -// GetDDLReorgHandle gets the latest processed DDL reorganize position. -func (m *Meta) GetDDLReorgHandle(job *model.Job) (element *Element, startKey, endKey kv.Key, physicalTableID int64, err error) { - elementBytes, err := m.txn.HGet(mDDLJobReorgKey, m.reorgJobCurrentElement(job.ID)) - if err != nil { - return nil, nil, nil, 0, errors.Trace(err) - } - if elementBytes == nil { - return nil, nil, nil, 0, ErrDDLReorgElementNotExist - } - element, err = DecodeElement(elementBytes) - if err != nil { - return nil, nil, nil, 0, errors.Trace(err) - } - - startKey, err = getReorgJobFieldHandle(m.txn, m.reorgJobStartHandle(job.ID, element)) - if err != nil { - return nil, nil, nil, 0, errors.Trace(err) - } - endKey, err = getReorgJobFieldHandle(m.txn, m.reorgJobEndHandle(job.ID, element)) - if err != nil { - return nil, nil, nil, 0, errors.Trace(err) - } - - physicalTableID, err = m.txn.HGetInt64(mDDLJobReorgKey, m.reorgJobPhysicalTableID(job.ID, element)) - if err != nil { - err = errors.Trace(err) - return - } - - // physicalTableID may be 0, because older version TiDB (without table partition) doesn't store them. - // update them to table's in this case. - if physicalTableID == 0 { - if job.ReorgMeta != nil { - endKey = kv.IntHandle(job.ReorgMeta.EndHandle).Encoded() - } else { - endKey = kv.IntHandle(math.MaxInt64).Encoded() - } - physicalTableID = job.TableID - logutil.BgLogger().Warn("new TiDB binary running on old TiDB DDL reorg data", - zap.Int64("partition ID", physicalTableID), - zap.Stringer("startHandle", startKey), - zap.Stringer("endHandle", endKey)) - } - return -} - -func getReorgJobFieldHandle(t *structure.TxStructure, reorgJobField []byte) (kv.Key, error) { - bs, err := t.HGet(mDDLJobReorgKey, reorgJobField) - if err != nil { - return nil, errors.Trace(err) - } - keyNotFound := bs == nil - if keyNotFound { - return nil, nil - } - return bs, nil -} - func (*Meta) schemaDiffKey(schemaVersion int64) []byte { return []byte(fmt.Sprintf("%s:%d", mSchemaDiffPrefix, schemaVersion)) } diff --git a/meta/meta_test.go b/meta/meta_test.go index 28603f76189ad..d1d932821bce3 100644 --- a/meta/meta_test.go +++ b/meta/meta_test.go @@ -17,18 +17,15 @@ package meta_test import ( "context" "fmt" - "math" "strconv" "testing" "time" "github.com/pingcap/errors" - "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/store/mockstore" - "github.com/pingcap/tidb/testkit/testutil" "github.com/pingcap/tidb/util" "github.com/stretchr/testify/require" ) @@ -448,202 +445,6 @@ func TestElement(t *testing.T) { require.EqualError(t, err, `invalid encoded element "_col_" length 5`) } -func TestDDL(t *testing.T) { - testCases := []struct { - desc string - startHandle kv.Handle - endHandle kv.Handle - }{ - { - "kv.IntHandle", - kv.IntHandle(1), - kv.IntHandle(2), - }, - { - "kv.CommonHandle", - testutil.MustNewCommonHandle(t, "abc", 1222, "string"), - testutil.MustNewCommonHandle(t, "dddd", 1222, "string"), - }, - } - - for _, tc := range testCases { - // copy iterator variable into a new variable, see issue #27779 - tc := tc - t.Run(tc.desc, func(t *testing.T) { - store, err := mockstore.NewMockStore() - require.NoError(t, err) - defer func() { - err := store.Close() - require.NoError(t, err) - }() - - txn, err := store.Begin() - require.NoError(t, err) - - m := meta.NewMeta(txn) - - job := &model.Job{ID: 1} - err = m.EnQueueDDLJob(job) - require.NoError(t, err) - n, err := m.DDLJobQueueLen() - require.NoError(t, err) - require.Equal(t, int64(1), n) - - v, err := m.GetDDLJobByIdx(0) - require.NoError(t, err) - require.Equal(t, job, v) - v, err = m.GetDDLJobByIdx(1) - require.NoError(t, err) - require.Nil(t, v) - - job.ID = 2 - err = m.UpdateDDLJob(0, job, true) - require.NoError(t, err) - - element := &meta.Element{ID: 123, TypeKey: meta.IndexElementKey} - // There are 3 meta key relate to index reorganization: - // start_handle, end_handle and physical_table_id. - // Only start_handle is initialized. - err = m.UpdateDDLReorgStartHandle(job, element, kv.IntHandle(1).Encoded()) - require.NoError(t, err) - - // Since physical_table_id is uninitialized, we simulate older TiDB version that doesn't store them. - // In this case GetDDLReorgHandle always return maxInt64 as end_handle. - e, i, j, k, err := m.GetDDLReorgHandle(job) - require.NoError(t, err) - require.Equal(t, element, e) - require.Equal(t, kv.Key(kv.IntHandle(1).Encoded()), i) - require.Equal(t, kv.Key(kv.IntHandle(math.MaxInt64).Encoded()), j) - require.Equal(t, int64(0), k) - - element = &meta.Element{ID: 222, TypeKey: meta.ColumnElementKey} - err = m.UpdateDDLReorgHandle(job.ID, tc.startHandle.Encoded(), tc.endHandle.Encoded(), 3, element) - require.NoError(t, err) - element1 := &meta.Element{ID: 223, TypeKey: meta.IndexElementKey} - err = m.UpdateDDLReorgHandle(job.ID, tc.startHandle.Encoded(), tc.endHandle.Encoded(), 3, element1) - require.NoError(t, err) - - e, i, j, k, err = m.GetDDLReorgHandle(job) - require.NoError(t, err) - require.Equal(t, element1, e) - require.Equal(t, kv.Key(tc.startHandle.Encoded()), i) - require.Equal(t, kv.Key(tc.endHandle.Encoded()), j) - require.Equal(t, int64(3), k) - - err = m.RemoveDDLReorgHandle(job, []*meta.Element{element, element1}) - require.NoError(t, err) - e, i, j, k, err = m.GetDDLReorgHandle(job) - require.True(t, meta.ErrDDLReorgElementNotExist.Equal(err)) - require.Nil(t, e) - require.Nil(t, i) - require.Nil(t, j) - require.Equal(t, k, int64(0)) - - // new TiDB binary running on old TiDB DDL reorg data. - e, i, j, k, err = m.GetDDLReorgHandle(job) - require.True(t, meta.ErrDDLReorgElementNotExist.Equal(err)) - require.Nil(t, e) - require.Nil(t, i) - require.Nil(t, j) - require.Equal(t, k, int64(0)) - - // Test GetDDLReorgHandle failed. - _, _, _, _, err = m.GetDDLReorgHandle(job) - require.True(t, meta.ErrDDLReorgElementNotExist.Equal(err)) - - v, err = m.DeQueueDDLJob() - require.NoError(t, err) - require.Equal(t, job, v) - - err = m.AddHistoryDDLJob(job, true) - require.NoError(t, err) - v, err = m.GetHistoryDDLJob(2) - require.NoError(t, err) - require.Equal(t, job, v) - - // Add multiple history jobs. - arg := "test arg" - historyJob1 := &model.Job{ID: 1234} - historyJob1.Args = append(job.Args, arg) - err = m.AddHistoryDDLJob(historyJob1, true) - require.NoError(t, err) - historyJob2 := &model.Job{ID: 123} - historyJob2.Args = append(job.Args, arg) - err = m.AddHistoryDDLJob(historyJob2, false) - require.NoError(t, err) - all, err := ddl.GetAllHistoryDDLJobs(m) - require.NoError(t, err) - var lastID int64 - for _, job := range all { - require.Greater(t, job.ID, lastID) - lastID = job.ID - arg1 := "" - err := job.DecodeArgs(&arg1) - require.NoError(t, err) - if job.ID == historyJob1.ID { - require.Equal(t, historyJob1.Args[0], *(job.Args[0].(*string))) - } else { - require.Len(t, job.Args, 0) - } - } - - // Test for get last N history ddl jobs. - historyJobs, err := ddl.GetLastNHistoryDDLJobs(m, 2) - require.NoError(t, err) - require.Len(t, historyJobs, 2) - require.Equal(t, int64(1234), historyJobs[0].ID) - require.Equal(t, int64(123), historyJobs[1].ID) - - // Test GetAllDDLJobsInQueue. - err = m.EnQueueDDLJob(job) - require.NoError(t, err) - job1 := &model.Job{ID: 2} - err = m.EnQueueDDLJob(job1) - require.NoError(t, err) - jobs, err := m.GetAllDDLJobsInQueue() - require.NoError(t, err) - expectJobs := []*model.Job{job, job1} - require.Equal(t, expectJobs, jobs) - - err = txn.Commit(context.Background()) - require.NoError(t, err) - }) - } -} - -func TestAddIndexJob(t *testing.T) { - store, err := mockstore.NewMockStore() - require.NoError(t, err) - defer func() { - err := store.Close() - require.NoError(t, err) - }() - - txn1, err := store.Begin() - require.NoError(t, err) - - m := meta.NewMeta(txn1, meta.AddIndexJobListKey) - job := &model.Job{ID: 1} - err = m.EnQueueDDLJob(job) - require.NoError(t, err) - job.ID = 123 - err = m.UpdateDDLJob(0, job, true, meta.AddIndexJobListKey) - require.NoError(t, err) - v, err := m.GetDDLJobByIdx(0, meta.AddIndexJobListKey) - require.NoError(t, err) - require.Equal(t, job, v) - l, err := m.DDLJobQueueLen(meta.AddIndexJobListKey) - require.NoError(t, err) - require.Equal(t, int64(1), l) - jobs, err := m.GetAllDDLJobsInQueue(meta.AddIndexJobListKey) - require.NoError(t, err) - expectJobs := []*model.Job{job} - require.Equal(t, expectJobs, jobs) - - err = txn1.Commit(context.Background()) - require.NoError(t, err) -} - func BenchmarkGenGlobalIDs(b *testing.B) { store, err := mockstore.NewMockStore() require.NoError(b, err) @@ -773,45 +574,6 @@ func TestSequenceKey(b *testing.T) { require.Equal(b, tableID, id) } -func TestClearJob(t *testing.T) { - store, err := mockstore.NewMockStore() - require.NoError(t, err) - defer func() { - require.NoError(t, store.Close()) - }() - - txn, err := store.Begin() - require.NoError(t, err) - - job1 := &model.Job{ID: 1, TableID: 1, Type: model.ActionAddColumn} - job2 := &model.Job{ID: 2, TableID: 1, Type: model.ActionCreateTable} - job3 := &model.Job{ID: 3, TableID: 2, Type: model.ActionDropColumn} - - m := meta.NewMeta(txn) - - require.NoError(t, m.EnQueueDDLJob(job1)) - require.NoError(t, m.EnQueueDDLJob(job2)) - require.NoError(t, m.EnQueueDDLJob(job3)) - - require.NoError(t, m.AddHistoryDDLJob(job1, false)) - require.NoError(t, m.AddHistoryDDLJob(job2, false)) - - jobs, err := m.GetAllDDLJobsInQueue() - require.NoError(t, err) - require.Len(t, jobs, 3) - require.NoError(t, m.ClearALLDDLJob()) - jobs, err = m.GetAllDDLJobsInQueue() - require.NoError(t, err) - require.Len(t, jobs, 0) - - count, err := m.GetHistoryDDLCount() - require.NoError(t, err) - require.Equal(t, count, uint64(2)) - - err = txn.Rollback() - require.NoError(t, err) -} - func TestCreateMySQLDatabase(t *testing.T) { store, err := mockstore.NewMockStore() require.NoError(t, err) @@ -835,41 +597,3 @@ func TestCreateMySQLDatabase(t *testing.T) { err = txn.Rollback() require.NoError(t, err) } - -func TestDDLTable(t *testing.T) { - store, err := mockstore.NewMockStore() - require.NoError(t, err) - defer func() { - require.NoError(t, store.Close()) - }() - - txn, err := store.Begin() - require.NoError(t, err) - - m := meta.NewMeta(txn) - - exists, err := m.CheckDDLTableExists() - require.NoError(t, err) - require.False(t, exists) - - err = m.SetDDLTables() - require.NoError(t, err) - - exists, err = m.CheckDDLTableExists() - require.NoError(t, err) - require.True(t, exists) - - err = m.SetConcurrentDDL(true) - require.NoError(t, err) - b, err := m.IsConcurrentDDL() - require.NoError(t, err) - require.True(t, b) - err = m.SetConcurrentDDL(false) - require.NoError(t, err) - b, err = m.IsConcurrentDDL() - require.NoError(t, err) - require.False(t, b) - - err = txn.Rollback() - require.NoError(t, err) -} diff --git a/metrics/meta.go b/metrics/meta.go index 519ba6a0924a1..af967fe48a3bb 100644 --- a/metrics/meta.go +++ b/metrics/meta.go @@ -34,8 +34,6 @@ var ( GetSchemaDiff = "get_schema_diff" SetSchemaDiff = "set_schema_diff" - GetDDLJobByIdx = "get_ddl_job" - UpdateDDLJob = "update_ddl_job" GetHistoryDDLJob = "get_history_ddl_job" MetaHistogram = prometheus.NewHistogramVec( diff --git a/parser/ast/ddl_test.go b/parser/ast/ddl_test.go index 156a66398426f..dbbb212037db8 100644 --- a/parser/ast/ddl_test.go +++ b/parser/ast/ddl_test.go @@ -248,6 +248,21 @@ func TestDDLColumnOptionRestore(t *testing.T) { runNodeRestoreTest(t, testCases, "CREATE TABLE child (id INT %s)", extractNodeFunc) } +func TestGeneratedRestore(t *testing.T) { + testCases := []NodeRestoreTestCase{ + {"generated always as(id + 1)", "GENERATED ALWAYS AS(`id`+1) VIRTUAL"}, + {"generated always as(id + 1) virtual", "GENERATED ALWAYS AS(`id`+1) VIRTUAL"}, + {"generated always as(id + 1) stored", "GENERATED ALWAYS AS(`id`+1) STORED"}, + {"generated always as(lower(id)) stored", "GENERATED ALWAYS AS(LOWER(`id`)) STORED"}, + {"generated always as(lower(child.id)) stored", "GENERATED ALWAYS AS(LOWER(`id`)) STORED"}, + } + extractNodeFunc := func(node Node) Node { + return node.(*CreateTableStmt).Cols[0].Options[0] + } + runNodeRestoreTestWithFlagsStmtChange(t, testCases, "CREATE TABLE child (id INT %s)", extractNodeFunc, + format.DefaultRestoreFlags|format.RestoreWithoutSchemaName|format.RestoreWithoutTableName) +} + func TestDDLColumnDefRestore(t *testing.T) { testCases := []NodeRestoreTestCase{ // for type diff --git a/parser/ast/dml.go b/parser/ast/dml.go index c711da90d123f..4e97ae8d95882 100644 --- a/parser/ast/dml.go +++ b/parser/ast/dml.go @@ -288,15 +288,17 @@ func (*TableName) resultSet() {} // Restore implements Node interface. func (n *TableName) restoreName(ctx *format.RestoreCtx) { - // restore db name - if n.Schema.String() != "" { - ctx.WriteName(n.Schema.String()) - ctx.WritePlain(".") - } else if ctx.DefaultDB != "" { - // Try CTE, for a CTE table name, we shouldn't write the database name. - if !ctx.IsCTETableName(n.Name.L) { - ctx.WriteName(ctx.DefaultDB) + if !ctx.Flags.HasWithoutSchemaNameFlag() { + // restore db name + if n.Schema.String() != "" { + ctx.WriteName(n.Schema.String()) ctx.WritePlain(".") + } else if ctx.DefaultDB != "" { + // Try CTE, for a CTE table name, we shouldn't write the database name. + if !ctx.IsCTETableName(n.Name.L) { + ctx.WriteName(ctx.DefaultDB) + ctx.WritePlain(".") + } } } // restore table name diff --git a/parser/ast/expressions.go b/parser/ast/expressions.go index 270c46218af61..6bca49f4d2a7d 100644 --- a/parser/ast/expressions.go +++ b/parser/ast/expressions.go @@ -512,11 +512,11 @@ type ColumnName struct { // Restore implements Node interface. func (n *ColumnName) Restore(ctx *format.RestoreCtx) error { - if n.Schema.O != "" && !ctx.IsCTETableName(n.Table.L) { + if n.Schema.O != "" && !ctx.IsCTETableName(n.Table.L) && !ctx.Flags.HasWithoutSchemaNameFlag() { ctx.WriteName(n.Schema.O) ctx.WritePlain(".") } - if n.Table.O != "" { + if n.Table.O != "" && !ctx.Flags.HasWithoutTableNameFlag() { ctx.WriteName(n.Table.O) ctx.WritePlain(".") } diff --git a/parser/format/format.go b/parser/format/format.go index 284d4dff4e9df..a60c8d7b6589d 100644 --- a/parser/format/format.go +++ b/parser/format/format.go @@ -236,6 +236,8 @@ const ( RestoreTiDBSpecialComment SkipPlacementRuleForRestore RestoreWithTTLEnableOff + RestoreWithoutSchemaName + RestoreWithoutTableName ) const ( @@ -247,6 +249,16 @@ func (rf RestoreFlags) has(flag RestoreFlags) bool { return rf&flag != 0 } +// HasWithoutSchemaNameFlag returns a boolean indicating when `rf` has `RestoreWithoutSchemaName` flag. +func (rf RestoreFlags) HasWithoutSchemaNameFlag() bool { + return rf.has(RestoreWithoutSchemaName) +} + +// HasWithoutTableNameFlag returns a boolean indicating when `rf` has `RestoreWithoutTableName` flag. +func (rf RestoreFlags) HasWithoutTableNameFlag() bool { + return rf.has(RestoreWithoutTableName) +} + // HasStringSingleQuotesFlag returns a boolean indicating when `rf` has `RestoreStringSingleQuotes` flag. func (rf RestoreFlags) HasStringSingleQuotesFlag() bool { return rf.has(RestoreStringSingleQuotes) diff --git a/roadmap.md b/roadmap.md index d125b4ae13ee6..a8b57db5440df 100644 --- a/roadmap.md +++ b/roadmap.md @@ -21,7 +21,7 @@ This roadmap brings you what's coming in the 1-year future, so you can see the n