github-actions[bot] commented on code in PR #63514:
URL: https://github.com/apache/doris/pull/63514#discussion_r3295772467
##########
fs_brokers/cdc_client/src/main/java/org/apache/doris/cdcclient/source/reader/mysql/MySqlSourceReader.java:
##########
@@ -1127,9 +1124,8 @@ private Map<TableId, TableChanges.TableChange>
discoverTableSchemas(JobBaseConfi
}
@Override
- public void close(JobBaseConfig jobConfig) {
+ public synchronized void close(JobBaseConfig jobConfig) {
LOG.info("Close source reader for job {}", jobConfig.getJobId());
Review Comment:
Same remaining race on the MySQL binlog path: `pollRecords()` no longer
holds the monitor, but this synchronized `close()` clears `binlogReader`,
`binlogSplit`, and `binlogSplitState` via `finishSplitRecords()` while
`pollRecordsFromBinlogReader()` reads those fields directly. If DROP/cancel
runs after the poller selects the binlog branch, the poller can observe
`binlogReader == null` or lose `binlogSplit` before constructing
`SplitRecords`, causing cleanup to surface as a failed poll/NPE. Please use
stable local references for the binlog reader/split/state during a poll, or
protect the handoff so close cannot null them until the poller is no longer
using them.
##########
fs_brokers/cdc_client/src/main/java/org/apache/doris/cdcclient/source/reader/JdbcIncrementalSourceReader.java:
##########
@@ -920,19 +918,9 @@ protected abstract Map<TableId, TableChanges.TableChange>
discoverTableSchemas(
JobBaseConfig config);
@Override
- public void close(JobBaseConfig jobConfig) {
+ public synchronized void close(JobBaseConfig jobConfig) {
LOG.info("Close source reader for job {}", jobConfig.getJobId());
Review Comment:
`pollRecords()` is now monitor-free, but the stream path still reads
`streamReader`, `streamSplit`, and `streamSplitState` directly while this
synchronized `close()` can run concurrently and `finishSplitRecords()` sets
them to `null`. A DROP/cancel can hit the window after `pollRecords()` chooses
the stream branch but before/during `pollRecordsFromStreamReader()`: the
precondition can fail with `streamReader is null`, or the call can dereference
a null reader/split after close has cleared the fields. That turns cancellation
into a failed poll rather than a deterministic close. Please make the stream
poll use a stable local snapshot of the reader/split/state for the whole poll,
or otherwise synchronize the field handoff so close cannot null the fields
while the poller is using them.
--
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]
