sandynz commented on PR #38679:
URL: https://github.com/apache/shardingsphere/pull/38679#issuecomment-4411615788
Merge Verdict: Mergeable
Reviewed Scope: latest two commits ad06af7d8fb and efe3f1166d2;
kernel/data-pipeline/dialect/mysql/src/main/java/.../MySQLIncrementalPositionManager.java;
MySQLIncrementalPositionManagerTest.java; MySQL 8.4 and MariaDB command
compatibility; local smoke logs.
Not Reviewed Scope: full E2E, real MariaDB position initialization with
log_bin=ON.
Need Expert Review: No.
Basis
The root cause is directly addressed. MySQL 8.4 no longer supports SHOW
MASTER STATUS; the official MySQL 8.4 manual says it is replaced by SHOW BINARY
LOG STATUS, and the replacement statement
returns binary log file/position status:
- https://dev.mysql.com/doc/refman/8.4/en/show-master-status.html
- https://dev.mysql.com/doc/refman/8.4/en/show-binary-log-status.html
The follow-up commit also handles the MySQL-family counterexample. MariaDB
documents SHOW [MASTER | BINLOG] STATUS, not MySQL’s SHOW BINARY LOG STATUS, so
gating the new command by
DatabaseMetaData.getDatabaseProductName() == "MySQL" avoids the trunk-SPI
fallback regression:
-
https://mariadb.com/docs/server/reference/sql-statements/administrative-sql-statements/show/show-binlog-status
Risk Assessment
- Design: acceptable. The version/product decision stays local to MySQL
binlog position initialization.
- Performance: negligible. One metadata product-name/version check during
position initialization only.
- Compatibility: MySQL < 8.4 keeps SHOW MASTER STATUS; MySQL 8.4+ uses
SHOW BINARY LOG STATUS; MariaDB keeps the old compatible path.
- Regression surface: covered by unit tests for MySQL 8.3, MySQL 8.4,
MySQL 9.0, and MariaDB 11.4 counterexample.
- Adversarial pass: the prior MariaDB regression path was found and fixed;
config-disabled path is not applicable; original MySQL 8.4 symptom is covered
by unit and real-instance smoke.
Validation
- Unit + dependencies:
- ./mvnw -pl kernel/data-pipeline/dialect/mysql -am -DskipITs
-Dspotless.skip=true -Dtest=MySQLIncrementalPositionManagerTest
-Dsurefire.failIfNoSpecifiedTests=false test
- Passed: Tests run: 6, Failures: 0, Errors: 0
- Style:
- spotless:apply: passed
- spotless:check checkstyle:check: passed
- Real smoke:
- MySQL 8.4 on 33161: Java position init returned mysql-bin.000002#158.
- MySQL 8.4 rejected SHOW MASTER STATUS, confirming the original
failure path.
- MariaDB 10.6.19 on 33561: rejected SHOW BINARY LOG STATUS; JDBC
metadata returned MariaDB; Java path did not fail with MySQL syntax. It later
failed because container log_bin=OFF, so no
position row existed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
