danny0405 commented on code in PR #19376:
URL: https://github.com/apache/hudi/pull/19376#discussion_r3655593668
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/prune/PartitionPruners.java:
##########
@@ -138,6 +150,13 @@ public Set<String> filter(Collection<String> partitions) {
return partitions.stream()
.filter(this.partitions::contains).collect(Collectors.toSet());
}
+
+ @Override
+ public String scopeToken() {
+ // Sorted so the token is order-independent: the same partition set
always yields the same
+ // token regardless of insertion order.
+ return "static(" +
this.partitions.stream().sorted().collect(Collectors.joining(",")) + ")";
Review Comment:
This encoding is not collision-free for valid partition names: `{ "a,b", "c"
}` and `{ "a", "b,c" }` both produce `static(a,b,c)`. A restore that changes
between those sets would therefore be accepted with stale splits, violating the
method's stated requirement that every selection change alter the token. Could
we use an unambiguous encoding (for example, length-prefix each sorted value)
or hash a canonical serialization?
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/source/enumerator/HoodieEnumeratorStateSerializer.java:
##########
@@ -40,7 +40,7 @@
*/
@Internal
public class HoodieEnumeratorStateSerializer implements
SimpleVersionedSerializer<HoodieSplitEnumeratorState> {
- private static final int VERSION = 1;
+ private static final int VERSION = 2;
Review Comment:
Bumping the outer state serializer to v2 also makes the unchanged call below
use `splitSerializer.deserialize(2, splitBytes)`, even though these bytes are
still produced by `HoodieSourceSplitSerializer` v1. The nested deserializer
currently ignores its version argument, so the tests pass, but this silently
couples the two independent formats and will select the wrong split format as
soon as that deserializer becomes version-aware. Could we record the nested
serializer version with each payload (or explicitly map outer v1/v2 to split
v1) instead of forwarding the outer version?
--
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]
