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.

Sign up for GitHub

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

[FLINK-34689][MySQL][Feature] check binlog_row_value_options #3148

Merged
merged 3 commits into from
Apr 12, 2024
Merged

[FLINK-34689][MySQL][Feature] check binlog_row_value_options #3148

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36eaa8f
ADD: implemented checkBinlogRowValueOptions within MySqlValidator
Mar 15, 2024
ae1d27a
EDIT: updated log message for checkBinlogRowValueOptions
Apr 10, 2024
7efd972
EDIT: renamed BINLOG_ROW_VALUE_OPTIONS -> DEFAULT_BINLOG_ROW_VALUE_OP…
Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions ...nnector-mysql-cdc/src/main/java/org/apache/flink/cdc/connectors/mysql/MySqlValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class MySqlValidator implements Validator {

private static final String BINLOG_FORMAT_ROW = "ROW";
private static final String BINLOG_FORMAT_IMAGE_FULL = "FULL";
Comment on lines 51 to 52
Copy link
Contributor Author

@SML0127 SML0127 Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@ruanhang1993
How about changing the variable names BINLOG_FORMAT_ROW, BINLOG_FORMAT_IMAGE_FULL to DEFAULT_BINLOG_FORMAT, DEFAULT_BINLOG_ROW_FORMAT?

Suggested change
private static final String BINLOG_FORMAT_ROW = "ROW";
private static final String BINLOG_FORMAT_IMAGE_FULL = "FULL";
private static final String DEFAULT_BINLOG_FORMAT = "ROW";
private static final String DEFAULT_BINLOG_ROW_FORMAT = "FULL";

In mysql 8, default values for the binlog_format and binlog_row_image options are ROW and full, respectively

Copy link
Contributor

@ruanhang1993 ruanhang1993 Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, @SML0127 .
I think we could not change these fields' names. These settings are checked by the ROW or FULL values.
But the new DEFAULT_BINLOG_ROW_VALUE_OPTIONS is used as the default value for mysql 5.7. And it is to check whether this setting is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your explanation, @ruanhang1993
I undertand your responses. Do you have any other suggestions?

private static final String BINLOG_ROW_VALUE_OPTIONS = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static final String BINLOG_ROW_VALUE_OPTIONS = "";
private static final String DEFAULT_BINLOG_ROW_VALUE_OPTIONS = "";


private final Properties dbzProperties;
private final MySqlSourceConfig sourceConfig;
Expand All @@ -70,6 +71,7 @@ public void validate() {
checkVersion(connection);
checkBinlogFormat(connection);
checkBinlogRowImage(connection);
checkBinlogRowValueOptions(connection);
checkTimeZone(connection);
} catch (SQLException ex) {
throw new TableException(
Expand Down Expand Up @@ -159,6 +161,27 @@ private void checkBinlogRowImage(JdbcConnection connection) throws SQLException
}
}

/** Check whether the binlog row value options is empty. */
private void checkBinlogRowValueOptions(JdbcConnection connection) throws SQLException {
String rowValueOptions =
connection
.queryAndMap(
"SHOW GLOBAL VARIABLES LIKE 'binlog_row_value_options'",
rs -> rs.next() ? rs.getString(2) : "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rs -> rs.next() ? rs.getString(2) : "")
rs -> rs.next() ? rs.getString(2) : DEFAULT_BINLOG_ROW_VALUE_OPTIONS)

.trim()
.toUpperCase();
// This setting was introduced in MySQL 8.0+ with default of empty string ''
// For older versions, assume empty string ''
if (!rowValueOptions.equals(BINLOG_ROW_VALUE_OPTIONS)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!rowValueOptions.equals(BINLOG_ROW_VALUE_OPTIONS)) {
if (!BINLOG_ROW_VALUE_OPTIONS.equals(rowValueOptions)) {

throw new ValidationException(
String.format(
"The MySQL server is configured with binlog_row_value_options %s rather than %s, which is "
+ "required for this connector to work properly. Change the MySQL configuration to use a "
+ "binlog_row_image='' and restart the connector.",
rowValueOptions, BINLOG_ROW_VALUE_OPTIONS));
Copy link
Contributor

@ruanhang1993 ruanhang1993 Apr 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this error message should be changed as follows.

The MySQL server is configured with binlog_row_value_options=%s, which is possible to cause losing some binlog events for the mysql cdc connector. Please remove the binlog_row_value_options setting in the MySQL server and rerun the job. See more details at https://dev.mysql.com/doc/refman/8.0/en/replication-features-json.html.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I do not understand wrong, we do not need to change the binlog_row_image setting. Am I right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do not understand wrong, we do not need to change the binlog_row_image setting. Am I right ?

@ruanhang1993
I updated log message.

Yes. we do not need to change both settings binlog_row_image and binlog_row_value_options.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rowValueOptions, BINLOG_ROW_VALUE_OPTIONS));
rowValueOptions));

}
}

/** Check whether the server timezone matches the configured timezone. */
private void checkTimeZone(JdbcConnection connection) throws SQLException {
String timeZoneProperty = dbzProperties.getProperty("database.serverTimezone");
Expand Down
Loading