sync-diff-inspector: improve checksum SQL with configurable algorithm and float handling

[joechenrh]

Cherry-pick of pingcap/tidb-tools#886

What problem does this PR solve?

Issue Number: close #12535

What is changed and how it works?

This PR makes two improvements to the checksum SQL used by sync-diff-inspector:

1. Configurable checksum algorithm (md5 / sha256): sync-diff-inspector previously hardcoded MD5 for chunk checksumming. In FIPS-compliant environments (TiDB built with ENABLE_FIPS=1), MD5 is disabled in the OpenSSL library used by TiKV, causing sync-diff-inspector to fail when TiKV's coprocessor tries to evaluate MD5 expressions. This adds a checksum-algorithm config flag with md5 (default) and sha256 options.

2. Replace round() with CAST(... AS DECIMAL(65, 30)) for float/double columns: The previous round(col, N-floor(log10(abs(col)))) normalization for float/double columns returned NULL when the column value was 0 (since log10(0) is undefined), causing silent data mismatches. Replacing with CAST(col AS DECIMAL(65, 30)) fixes this bug and also enables TiKV coprocessor pushdown since CAST ... AS DECIMAL is a supported coprocessor expression.

The checksum SQL is also refactored to use a subquery pattern (SELECT COUNT(*), BIT_XOR(...) FROM (SELECT hash_func(...) as hash FROM ...) as t) for cleaner generated SQL.

Check List

Tests

• Unit test

Questions

Will it cause performance regression or break compatibility?

No. Default checksum algorithm remains MD5 for backwards compatibility. The CAST ... AS DECIMAL change may improve performance by enabling TiKV coprocessor pushdown for float/double columns.

Do you need to update user documentation, design documentation or monitoring documentation?

No. The checksum-algorithm flag is hidden for now.

Release note

sync-diff-inspector: add `checksum-algorithm` config option to support FIPS-compliant environments (SHA256); replace `round()` with `CAST(... AS DECIMAL(65, 30))` for float/double columns to fix zero-value NULL bug and enable coprocessor pushdown

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign leoppro for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the sync-diff-inspector tool by providing flexibility in the choice of checksum algorithms. Previously, the tool was hardcoded to use MD5, which can be problematic in environments requiring Federal Information Processing Standards (FIPS) compliance. The changes introduce a new configuration option, allowing users to specify either MD5 (for compatibility) or SHA256, thereby broadening the tool's applicability to more secure and regulated operational contexts.

Highlights

• Configurable Checksum Algorithm: Introduced a new checksum-algorithm configuration flag to allow users to select between different hash functions for chunk checksumming.
• FIPS Compliance: Added support for SHA256 checksums, enabling sync-diff-inspector to operate in FIPS-compliant environments where MD5 might be disabled.
• Backwards Compatibility: Maintained MD5 as the default checksum algorithm to ensure backwards compatibility with existing configurations.
• Configuration Validation: Implemented validation for the checksum-algorithm configuration to ensure only 'md5' or 'sha256' are accepted values.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• sync_diff_inspector/config/config.go
  • Defined ChecksumAlgorithm type and constants for MD5 and SHA256.
  • Added ChecksumAlgorithm field to DataSource and Config structs.
  • Updated NewConfig to include a command-line flag for checksum-algorithm.
  • Modified adjustConfigByDMSubTasks to propagate the ChecksumAlgorithm to data sources.
  • Implemented validation and normalization for the ChecksumAlgorithm in the Init method.
• sync_diff_inspector/config/config_test.go
  • Updated TestParseConfig to reflect the new checksum-algorithm field in the JSON output.
  • Expanded TestError with new test cases to validate the checksum-algorithm configuration.
  • Renamed TestGetCountAndMD5Checksum to TestGetCountAndChecksumMD5.
• sync_diff_inspector/source/mysql_shard.go
  • Added checksumAlgorithm field to the MySQLSources struct.
  • Modified GetCountAndMD5 method to call the newly refactored utils.GetCountAndChecksum with the configured algorithm.
  • Updated NewMySQLSources to initialize the checksumAlgorithm for MySQL sources.
• sync_diff_inspector/source/tidb.go
  • Added checksumAlgorithm field to the TiDBSource struct.
  • Modified GetCountAndMD5 method to call the newly refactored utils.GetCountAndChecksum with the configured algorithm.
  • Updated NewTiDBSource to initialize the checksumAlgorithm for TiDB sources.
• sync_diff_inspector/utils/utils.go
  • Renamed GetCountAndMD5Checksum to GetCountAndChecksum.
  • Added checksumAlgorithm parameter to GetCountAndChecksum.
  • Modified the SQL query generation within GetCountAndChecksum to dynamically use MD5 or SHA2 based on the provided algorithm.
  • Updated comments to reflect support for both MD5 and SHA256 checksum examples.
• sync_diff_inspector/utils/utils_test.go
  • Renamed TestGetCountAndMD5Checksum to TestGetCountAndChecksumMD5.
  • Added a new test function TestGetCountAndChecksumSHA256 to verify SHA256 checksum calculation.

Activity

• No human activity (comments, reviews) was provided in the context for this pull request.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a configurable checksum algorithm, allowing the use of SHA256 in addition to the default MD5. This is a valuable change for FIPS-compliant environments. The implementation looks solid, with changes correctly propagated through configuration, data sources, and utility functions. I've added a couple of suggestions to improve maintainability and performance.

[joechenrh]

/retest

[joechenrh]

/retest

[wuhuizuo]

/test wip-pull-build

[wuhuizuo]

/test ?

[ti-chi-bot]

@wuhuizuo: The following commands are available to trigger required jobs:

/test pull-cdc-integration-kafka-test

/test pull-cdc-integration-mysql-test

/test pull-cdc-integration-pulsar-test

/test pull-cdc-integration-storage-test

/test pull-dm-compatibility-test

/test pull-dm-integration-test

/test pull-error-log-review

/test pull-syncdiff-integration-test

/test pull-verify

/test wip-pull-build

/test wip-pull-check

/test wip-pull-unit-test-cdc

/test wip-pull-unit-test-dm

/test wip-pull-unit-test-engine

Use /test all to run the following jobs that were automatically triggered:

pingcap/tiflow/ghpr_verify

pingcap/tiflow/pull_cdc_integration_kafka_test

pingcap/tiflow/pull_cdc_integration_pulsar_test

pingcap/tiflow/pull_cdc_integration_storage_test

pingcap/tiflow/pull_cdc_integration_test

pingcap/tiflow/pull_dm_compatibility_test

pingcap/tiflow/pull_dm_integration_test

pingcap/tiflow/pull_syncdiff_integration_test

pull-error-log-review

Details

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[wuhuizuo]

/test wip-pull-check wip-pull-unit-test-cdc wip-pull-unit-test-dm wip-pull-unit-test-engine

[wuhuizuo]

/test wip-pull-unit-test-engine wip-pull-unit-test-dm

[ti-chi-bot]

@joechenrh: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
wip-pull-unit-test-engine	b04de69	link	true	/test wip-pull-unit-test-engine
wip-pull-unit-test-dm	b04de69	link	true	/test wip-pull-unit-test-dm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.3609%. Comparing base (8879687) to head (4aa92c9).
⚠️ Report is 5 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Components	Coverage Δ
cdc	57.3117% <ø> (-0.0507%)	⬇️
dm	49.1041% <ø> (+0.1340%)	⬆️
engine	50.6602% <ø> (-0.0847%)	⬇️

Flag	Coverage Δ
cdc	57.3117% <ø> (?)
unit	53.3609% <ø> (-0.0740%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master     #12533        +/-   ##
================================================
- Coverage   53.4349%   53.3609%   -0.0740%     
================================================
  Files          1033       1010        -23     
  Lines        144458     139870      -4588     
================================================
- Hits          77191      74636      -2555     
+ Misses        61495      59624      -1871     
+ Partials       5772       5610       -162     

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.