Closed michaelmdeng closed 2 weeks ago
Timeline:
2024-08-09 03:08:29.842323853 +0000 UTC m=+580639.709422926
: :ballot_box_with_check: agreed by Leavrth./merge
@Leavrth: We have migrated to builtin LGTM
and approve
plugins for reviewing.
👉 Please use /approve
when you want approve this pull request.
The changes announcement: LGTM plugin changes
/test all
/test unit-test
@Leavrth: The specified target(s) for /test
were not found.
The following commands are available to trigger required jobs:
/test pull-verify
Use /test all
to run all jobs.
/test pull-unit-test
@Leavrth: The specified target(s) for /test
were not found.
The following commands are available to trigger required jobs:
/test pull-verify
Use /test all
to run all jobs.
/test pull-verify
/LGTM
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Leavrth
The full list of commands accepted by this bot can be found here.
The pull request process is described here
/ok-to-test
What problem does this PR solve?
Issue Number: close #634 #703
What is changed and how it works?
Change checksum algorithm to use md5 instead of crc32 to minimize chance of collision.
Essentially a copy of #707 that addresses merge conflicts and simplifies md5 checksum. Since md5 produces a 128-bit checksum, previous iteration attempted to track the checksum as two uint64s for lhs/rhs. This change simplifies the checksum query to bit_xor the lhs and the rhs into a single uint64 output.
Check List
Tests
Code changes
Side effects
Some previous concerns about pingcap/tidb#39576 and optimizing plan for checksum query rather than performing the same checksum operation twice for each lhs/rhs. Here's the explain output of the checksum query against a test table. I believe this shows that planner is pushing down the checksum functions down to the tikv coprocessor.
Related changes