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
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.New("detected the old version of tidb cluster")
tangenta marked this conversation as resolved.
Show resolved Hide resolved
}
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)
reorgInfo, err := getReorgInfo(d.jobContext(job), d, rh, job, tbl, BuildElements(changingCol, changingIdxs), false)
if err != nil || reorgInfo.first {
// If we run reorg firstly, we should update the job snapshot version
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