github-actions[bot] commented on code in PR #64938:
URL: https://github.com/apache/doris/pull/64938#discussion_r3503340268
##########
fs_brokers/cdc_client/src/main/java/org/apache/doris/cdcclient/service/PipelineCoordinator.java:
##########
@@ -589,13 +589,13 @@ public void writeRecords(WriteRecordRequest
writeRecordRequest) throws Exception
&& maxIntervalMillis > 0
&& elapsedTime >= maxIntervalMillis;
- if (!isSnapshotSplit && timeoutReached) {
+ if (!isSnapshotSplit && timeoutReached && !shouldStop)
{
Review Comment:
This new drain behavior needs a regression test for the production
`reuseReader=true` binlog path. FE sets `reuseReader` for binlog splits, but
`WriteRecordRequest` defaults it to false and the existing CDC write harness
builds requests without calling `setReuseReader`; I also do not see a
heartbeat-plus-data test covering this branch. Because the bug this fixes
depends on records after the heartbeat being already dequeued and then lost
only when the reader is reused, the current tests would not fail if this went
back to breaking at the heartbeat. Please add a focused test that drives a
timed-out binlog batch containing a heartbeat followed by data with
`reuseReader=true`, and asserts the post-heartbeat data is written and
committed before the task returns.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
