leonardBang commented on code in PR #4489:
URL: https://github.com/apache/flink-cdc/pull/4489#discussion_r4347985597
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-cdc-base/src/main/java/org/apache/flink/cdc/connectors/base/source/assigner/state/SnapshotPendingSplitsState.java:
##########
@@ -154,7 +154,7 @@ public List<SchemalessSnapshotSplit> getRemainingSplits() {
return remainingSplits;
}
- public Map<String, SchemalessSnapshotSplit> getAssignedSplits() {
+ public LinkedHashMap<String, SchemalessSnapshotSplit> getAssignedSplits() {
Review Comment:
Overall, the approach looks sound: preserving assignment order across
restore keeps meta-group IDs stable, and the reader-side prefix handling
follows that invariant.
One compatibility detail: changing the public
`SnapshotPendingSplitsState#getAssignedSplits()` return type from `Map` to
`LinkedHashMap` changes its JVM method descriptor (`()Ljava/util/Map;` to
`()Ljava/util/LinkedHashMap;`). An out-of-tree connector compiled against the
previous `flink-cdc-base` could therefore hit `NoSuchMethodError` after
upgrading. The 11-argument public constructor also narrows its `assignedSplits`
parameter from `Map` to `LinkedHashMap`, breaking existing source and binary
callers.
Could we keep those public signatures as `Map` and preserve the required
iteration order internally with a `LinkedHashMap` copy? A small compatibility
test using a caller compiled against the old signatures would help cover this
upgrade path.
--
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]
