wenshao commented on code in PR #4362:
URL: https://github.com/apache/flink-cdc/pull/4362#discussion_r3043297102
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-cdc-base/src/main/java/org/apache/flink/cdc/connectors/base/source/reader/IncrementalSourceReader.java:
##########
@@ -121,8 +122,12 @@ public IncrementalSourceReader(
super(
new SingleThreadFetcherManager<>(splitReaderSupplier::get),
recordEmitter,
+ null,
config,
- incrementalSourceReaderContext.getSourceReaderContext());
+ incrementalSourceReaderContext.getSourceReaderContext(),
+ sourceConfig.getRecordsPerSecond() == -1
+ ? null
+ :
RateLimiterStrategy.perSecond(sourceConfig.getRecordsPerSecond()));
Review Comment:
**[Suggestion]** Rate limiter applies to both snapshot AND
incremental/binlog phases without distinction
The `RateLimiterStrategy` is passed unconditionally to the source reader,
throttling ALL records regardless of split type. For CDC connectors this has a
notable behavioral risk:
- **Snapshot phase**: Rate limiting is typically desired to avoid
overwhelming the source database.
- **Incremental/binlog phase**: Rate limiting can cause the connector to
fall behind the upstream change stream, risking binlog/WAL purging before the
connector catches up (data loss for MySQL, replication slot issues for
PostgreSQL).
The option description and documentation do not warn about this risk.
Consider adding a documentation note warning users about the potential danger
of rate limiting during the incremental phase.
_— Qwen Code /review_
--
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]
