github-actions[bot] commented on code in PR #63480:
URL: https://github.com/apache/doris/pull/63480#discussion_r3286761335
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/DataSourceConfigValidator.java:
##########
@@ -109,8 +109,12 @@ public static void validateSource(Map<String, String>
input,
}
}
- // Cross-field: verify-ca must be paired with a CA cert; otherwise the
reader will
- // silently fall back to the JVM default truststore and likely fail to
connect.
+ validateSslVerifyCaPair(input);
Review Comment:
`validateSource()` is also used by `AlterJobCommand` with only the changed
source properties, not the full effective source configuration. With this
unconditional cross-field check, an existing job that already has
`ssl_rootcert` cannot be altered to set `ssl_mode=verify-ca` unless the user
redundantly includes the unchanged cert in the ALTER map, because
`validateSslVerifyCaPair(input)` only sees the delta and throws. Please run
this pair check on a merged existing+delta map in the ALTER path, or keep
`validateSource()` limited to per-key validation and add a separate
effective-config validation step.
##########
fe/fe-core/src/main/java/org/apache/doris/job/offset/jdbc/JdbcSourceOffsetProvider.java:
##########
@@ -87,7 +87,7 @@ public class JdbcSourceOffsetProvider implements
SourceOffsetProvider {
List<SnapshotSplit> remainingSplits = new ArrayList<>();
List<SnapshotSplit> finishedSplits = new ArrayList<>();
- JdbcOffset currentOffset;
+ volatile JdbcOffset currentOffset;
Review Comment:
Making `currentOffset` volatile publishes the reference to other threads,
but `updateOffset()` assigns it before snapshot splits are fully populated and
before `remainingSplits` / `finishedSplits` are updated under `splitsLock`. A
concurrent `getShowCurrentOffset()` can now serialize the just-published
`SnapshotSplit` before `tableId`, `splitKey`, and split bounds are copied, and
`hasMoreDataToConsume()` can also acquire `splitsLock` in that window and
reason about the old split lists with the new offset. Please build/mutate a
local `JdbcOffset` completely first and only assign to the volatile field after
the synchronized snapshot bookkeeping is complete (or keep all readers/writers
under the same lock).
--
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]
