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

improve the usability of DDL Job.Args #53930

Closed
26 tasks done
Tracked by #54436
lance6716 opened this issue Jun 11, 2024 · 1 comment
Closed
26 tasks done
Tracked by #54436

improve the usability of DDL Job.Args #53930

lance6716 opened this issue Jun 11, 2024 · 1 comment
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@lance6716
Copy link
Contributor

lance6716 commented Jun 11, 2024

Enhancement

Currently Job.Args is like a type-erasure field

// Note: it might change when state changes, such as when rollback on AddColumn.
// - CreateTable, it's [model.TableInfo, foreignKeyCheck]
// - AddIndex or AddPrimaryKey: [unique, ....
// - TruncateTable: [new-table-id, foreignKeyCheck, ...
// - RenameTable: [old-db-id, new-table-name, old-db-name]
// - ExchangeTablePartition: [partition-id, pt-db-id, pt-id, partition-name, with-validation]
Args []interface{} `json:"-"`
// RawArgs : We must use json raw message to delay parsing special args.
RawArgs json.RawMessage `json:"raw_args"`

It's hard to find all usage of a DDL job type arguments and maintain them.

image

idea 1: I think it's better to have a specialized struct for each DDL job type. So the developer can directly find all usage of a DDL job type among all types.

idea 2: don't change old format of the serialized Job.RawArgs, the new structs should handle the serialization/deserialization in the old format. The advantage is we don't need to handle the compatibility of DDL jobs written by old version clusters.

Tasks

@lance6716 lance6716 added the type/enhancement The issue or PR belongs to an enhancement. label Jun 11, 2024
@lance6716
Copy link
Contributor Author

And for one type of DDL jobs, the Args may change so the marshal field RawArgs is hard to maintain

For example, DROP DATABASE will have a bool as Args at first

tidb/pkg/ddl/ddl_api.go

Lines 685 to 687 in d19fc99

Type: model.ActionDropSchema,
BinlogInfo: &model.HistoryInfo{},
Args: []any{fkCheck},

but somehow it becomes []int64

case model.ActionDropSchema:
var tableIDs []int64
if err := job.DecodeArgs(&tableIDs); err != nil {

Benjamin2037 pushed a commit to Benjamin2037/tidb that referenced this issue Sep 11, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 19, 2024
winoros pushed a commit to winoros/tidb that referenced this issue Sep 23, 2024
winoros pushed a commit to winoros/tidb that referenced this issue Sep 23, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 24, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 24, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 25, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 26, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 27, 2024
ti-chi-bot bot pushed a commit that referenced this issue Sep 30, 2024
ti-chi-bot bot pushed a commit that referenced this issue Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants