Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: remove the support of the old ddl framework #39684

Merged
merged 17 commits into from
Dec 30, 2022
4 changes: 0 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Copy link
Contributor

@Defined2014 Defined2014 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

featuretag will also used by other feature. Please do not delete it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not enable it only for distributing reorg, so I remove it now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do u mean that we can add it back when it is used by other features?

--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
Expand Down
1 change: 0 additions & 1 deletion br/pkg/conn/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions br/pkg/version/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
5 changes: 1 addition & 4 deletions br/pkg/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
15 changes: 2 additions & 13 deletions br/pkg/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
}

Expand Down
2 changes: 1 addition & 1 deletion ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 0 additions & 26 deletions ddl/concurrentddltest/BUILD.bazel

This file was deleted.

45 changes: 0 additions & 45 deletions ddl/concurrentddltest/main_test.go

This file was deleted.

149 changes: 0 additions & 149 deletions ddl/concurrentddltest/switch_test.go

This file was deleted.

Loading