lvyanquan commented on code in PR #4362:
URL: https://github.com/apache/flink-cdc/pull/4362#discussion_r4398554990
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-postgres-cdc/src/main/java/org/apache/flink/cdc/connectors/postgres/table/PostgreSQLTableSource.java:
##########
@@ -168,6 +170,7 @@ public PostgreSQLTableSource(
this.appendOnly = appendOnly;
this.includePartitionedTables = includePartitionedTables;
this.releaseSnapshotMetadataEnabled = releaseSnapshotMetadataEnabled;
+ this.recordsPerSecond = recordsPerSecond;
Review Comment:
**PostgreSQL configuration forwarding**
The `records.per.second` option is parsed and stored, but it is not
forwarded to the incremental source builder, so PostgreSQL SQL sources silently
ignore the configured rate limit. Could we add
`.recordsPerSecond(recordsPerSecond)` and a test with a non-default value?
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-cdc-base/src/main/java/org/apache/flink/cdc/connectors/base/options/JdbcSourceOptions.java:
##########
@@ -110,4 +110,11 @@ public class JdbcSourceOptions extends SourceOptions {
.withDescription(
"The chunk key of table snapshot, captured tables
are split into multiple chunks by a chunk key when read the snapshot of table."
+ "By default, the chunk key is the first
column of the primary key and the chunk key is the RowId in oracle.");
+
+ public static final ConfigOption<Double> RECORDS_PER_SECOND =
+ ConfigOptions.key("records.per.second")
Review Comment:
Could we rename this option to `scan.rate-limit.records-per-second` to make
its scope and purpose clearer and align it with the existing `scan.*` options?
##########
docs/content/docs/connectors/flink-sources/mysql-cdc.md:
##########
@@ -481,6 +481,16 @@ Only valid for cdc 1.x version. During a snapshot
operation, the connector will
For example updating an already updated value in snapshot, or deleting
an already deleted entry in snapshot. These replayed change log events should
be handled specially.
</td>
</tr>
+ <tr>
+ <td>records.per.second</td>
+ <td>optional</td>
+ <td style="word-wrap: break-word;">true</td>
Review Comment:
**Documentation default value**
The new `records.per.second` entries list `true` as the default, while the
actual default is `-1` (no rate limiting). Could we update the default to `-1`
across the affected English and Chinese documentation?
--
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]
