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.

Sign up for GitHub

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

br: adjust default tidb global config for br #45794

Merged
merged 1 commit into from
Aug 17, 2023
+4 −0
Merged

br: adjust default tidb global config for br #45794

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions br/pkg/task/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,10 @@ func enableTiDBConfig() func() {
// when upstream and downstream both set this value greater than default(3072)
conf.MaxIndexLength = config.DefMaxOfMaxIndexLength
Copy link
Contributor

@lance6716 lance6716 Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

seems fail on same type of mistake again. Do you consider to skip checks when creating table? like

tidb/ddl/schematracker/dm_tracker.go

Line 178 in 522a008

func (d SchemaTracker) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) error {

Copy link
Contributor

Choose a reason for hiding this comment

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

tidb/ddl/schematracker/dm_tracker_test.go

Line 50 in 522a008

func TestNoNumLimit(t *testing.T) {

DM has met a similar problem before, which is upstream MySQL has more capability than TiDB, so we use a special DDL implementation to apply DDL SQL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a check before create table with DDL.

tidb/ddl/ddl_api.go

Lines 2337 to 2352 in 57d778f

func buildTableInfoWithCheck(ctx sessionctx.Context, s *ast.CreateTableStmt, dbCharset, dbCollate string, placementPolicyRef *model.PolicyRefInfo) (*model.TableInfo, error) {
tbInfo, err := BuildTableInfoWithStmt(ctx, s, dbCharset, dbCollate, placementPolicyRef)
if err != nil {
return nil, err
}
// Fix issue 17952 which will cause partition range expr can't be parsed as Int.
// checkTableInfoValidWithStmt will do the constant fold the partition expression first,
// then checkTableInfoValidExtra will pass the tableInfo check successfully.
if err = checkTableInfoValidWithStmt(ctx, tbInfo, s); err != nil {
return nil, err
}
if err = checkTableInfoValidExtra(tbInfo); err != nil {
return nil, err
}
return tbInfo, nil
}

When use br, every check config is default value. If upstream has large value, for example set TableColumnCountLimit to 4000, the restore might be failed when there is a table with 4000 columns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BR import the DDL package, but doesn't expose these configs.

log.Warn("set max-index-length to max(3072*4) to skip check index length in DDL")
conf.IndexLimit = config.DefMaxOfIndexLimit
log.Warn("set index-limit to max(64*8) to skip check index count in DDL")
conf.TableColumnCountLimit = config.DefMaxOfTableColumnCountLimit
log.Warn("set table-column-count to max(4096) to skip check column count in DDL")
})
return restoreConfig
}
Expand Down