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

Skip the tiflash job. #941

Merged
merged 6 commits into from
Apr 3, 2020
Merged

Skip the tiflash job. #941

merged 6 commits into from
Apr 3, 2020

Conversation

july2993
Copy link
Contributor

@july2993 july2993 commented Apr 3, 2020

What problem does this PR solve?

Fix abort when there'a model.ActionUpdateTiFlashReplicaStatus job caused the job.Query is empty:

return "", "", "", errors.Errorf("[ddl job sql miss]%+v", job)

[ddl job sql miss]ID:85, Type:update tiflash replica status, State:synced, SchemaState:public, SchemaID:45, TableID:62, RowCount:0, ArgLen:0, start time: 2020-04-01 10:52:07.845 +0800 CST, Err:<nil>, ErrCount:0, SnapshotVersion:0"] [errorVerbose="[ddl job sql miss]I      D:85, Type:update tiflash replica status, State:synced, SchemaState:public, SchemaID:45, TableID:62, RowCount:0, ArgLen:0, start time: 2020-04-01 10:52:07.845 +0800 CST, Err:<nil>, ErrCount:0, SnapshotVersion:0\ngithub.com/pingcap/tidb-binlog/drainer.(*Schema).handl      eDDL\n\t/home/jenkins/agent/workspace/release_tidb_4.0/go/src/github.com/pingcap/tidb-binlo

What is changed and how it works?

Skip the tiflash job.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    test:
  1. create table t(id int);
  2. ALTER TABLE t SET TIFLASH REPLICA 3 LOCATION LABELS "rack", "host", "abc"; // model.ActionSetTiFlashReplica
  3. curl -X POST -d '{"id":45,"region_count":3,"flash_region_count":3}' https://127.0.0.1:10080/tiflash/replica // model.ActionUpdateTiFlashReplicaStatus
  4. performance write on table t and check can still replicate.

ref: pingcap/tidb#12453

  • No code

Related changes

  • Need to cherry-pick to the release branch

  • Need to be included in the release note

    • Skip TiFlash relate DDL job to avoid replication abort.

test:
1. create table t(id int);
2. ALTER TABLE t SET TIFLASH REPLICA 3 LOCATION LABELS "rack", "host", "abc";            // model.ActionSetTiFlashReplica
3. curl -X POST -d '{"id":45,"region_count":3,"flash_region_count":3}' https://127.0.0.1:10080/tiflash/replica  // model.ActionUpdateTiFlashReplicaStatus
4. performance write on table t and check can still replicate.

ref: pingcap/tidb#12453
IANTHEREAL
IANTHEREAL previously approved these changes Apr 3, 2020
Copy link
Collaborator

@IANTHEREAL IANTHEREAL left a comment

Choose a reason for hiding this comment

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

LGTM

@IANTHEREAL IANTHEREAL self-requested a review April 3, 2020 07:05
@july2993
Copy link
Contributor Author

july2993 commented Apr 3, 2020

/run-all-tests

@july2993
Copy link
Contributor Author

july2993 commented Apr 3, 2020

/run-all-tests

switch job.Type {
case model.ActionUpdateTiFlashReplicaStatus: // empty job.Query
return true
case model.ActionSetTiFlashReplica:

Choose a reason for hiding this comment

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

No need to skip ActionSetTiFlashReplica DDL job? this DDL has Query, How about synchronize this DDL and ignore the error?

@july2993
Copy link
Contributor Author

july2993 commented Apr 3, 2020

/run-all-tests tidb=release-4.0 tikv=release-4.0 pd=release-4.0

@july2993
Copy link
Contributor Author

july2993 commented Apr 3, 2020

/run-all-tests tidb=release-4.0 tikv=release-4.0 pd=release-4.0

@july2993
Copy link
Contributor Author

july2993 commented Apr 3, 2020

/run-all-tests tidb=release-4.0 tikv=release-4.0 pd=release-4.0

Copy link
Collaborator

@IANTHEREAL IANTHEREAL left a comment

Choose a reason for hiding this comment

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

LGTM

@july2993 july2993 merged commit 405c939 into master Apr 3, 2020
@july2993 july2993 deleted the skip_flash branch April 3, 2020 10:33
@sre-bot
Copy link

sre-bot commented Apr 3, 2020

cherry pick to release-3.1 in PR #942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants