Skip to content

*: refactoring the code of batchChecker#12108

Merged
tiancaiamao merged 5 commits intopingcap:masterfrom
tiancaiamao:refactor-batch-checker
Sep 10, 2019
Merged

*: refactoring the code of batchChecker#12108
tiancaiamao merged 5 commits intopingcap:masterfrom
tiancaiamao:refactor-batch-checker

Conversation

@tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Sep 10, 2019

What problem does this PR solve?

The batchChecker is difficult to maintain, we should try to get rid of it.

CREATE TABLE `t` (
  `a` int(11) NOT NULL,
  `b` int(11) NOT NULL,
  `c` int(11) DEFAULT NULL,
  `d` int(11) DEFAULT NULL,
  `e` int(11) DEFAULT NULL,
  PRIMARY KEY (`a`,`b`),
  UNIQUE KEY `b` (`b`,`c`,`d`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
PARTITION BY RANGE ( `b` ) (
  PARTITION p0 VALUES LESS THAN (4),
  PARTITION p1 VALUES LESS THAN (7),
  PARTITION p2 VALUES LESS THAN (11)
) ;

insert into t values (1,2,3,4,5);
insert into t values (1,2,3,4,5),(6,2,3,4,6) on duplicate key update e = e + values(e);

Expect:

mysql> select * from t;
+---+---+------+------+------+
| a | b | c    | d    | e    |
+---+---+------+------+------+
| 1 | 2 |    3 |    4 |   16 |
+---+---+------+------+------+
1 row in set (0.00 sec)

Get:

mysql> select * from t;
+---+---+------+------+------+
| a | b | c    | d    | e    |
+---+---+------+------+------+
| 1 | 2 |    3 |    4 |   11 |
+---+---+------+------+------+
1 row in set (0.00 sec)

What is changed and how it works?

The first step: do not use it.
The second step: clean up the code.

In this commit, I catch the BatchGet result into the snapshot, in this way we can achieve the same goal as the batchChecker
I re-implement the insert ... on duplicate key update ... without using the batchChecker.

Check List

Tests

  • Unit test

Side effects

  • Possible performance regression (unlikely)

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Write release note for bug-fix or new feature.

Fix bugs of insert ... on duplicate update on the partitioned table

batchChecker is difficult to maintain, we should get rid of it.
In this commit I catch the BatchGet result into the snapshot, in this way we can
achieve the same goal as the batchChecker
@tiancaiamao tiancaiamao added type/enhancement The issue or PR belongs to an enhancement. type/bugfix This PR fixes a bug. require-LGT3 Indicates that the PR requires three LGTM. sig/execution SIG execution needs-cherry-pick-3.0 labels Sep 10, 2019
@tiancaiamao tiancaiamao changed the base branch from refactor-batch-checker to master September 10, 2019 03:14
@tiancaiamao
Copy link
Contributor Author

PTAL @jackysp @lysu @coocood

@codecov
Copy link

codecov bot commented Sep 10, 2019

Codecov Report

Merging #12108 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12108   +/-   ##
===========================================
  Coverage   81.5473%   81.5473%           
===========================================
  Files           449        449           
  Lines         97124      97124           
===========================================
  Hits          79202      79202           
  Misses        12317      12317           
  Partials       5605       5605

Copy link
Contributor

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Contributor

jackysp commented Sep 10, 2019

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/rebuild

Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM, so update, replace also need do same thing?

@lysu lysu added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 10, 2019
@tiancaiamao
Copy link
Contributor Author

Yes, if we want to remove the batchChecker totally. @lysu

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

Copy link

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

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

LGTM

@tiancaiamao tiancaiamao merged commit 64298f0 into pingcap:master Sep 10, 2019
@tiancaiamao tiancaiamao deleted the refactor-batch-checker branch September 10, 2019 08:36
@sre-bot
Copy link
Contributor

sre-bot commented Sep 10, 2019

cherry pick to release-3.0 failed

tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Sep 10, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-3.0. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @tiancaiamao PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

require-LGT3 Indicates that the PR requires three LGTM. sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/enhancement The issue or PR belongs to an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants