lvyanquan commented on code in PR #4498:
URL: https://github.com/apache/flink-cdc/pull/4498#discussion_r3851108940
##########
flink-cdc-common/src/main/java/org/apache/flink/cdc/common/function/HashFunction.java:
##########
@@ -28,4 +28,14 @@
public interface HashFunction<T> {
int hashcode(T event);
+
+ /**
+ * Calculates the hash code with the upstream source subtask index.
+ *
+ * <p>Implementations that do not depend on the source subtask index can
continue implementing
+ * {@link #hashcode(Object)} only.
+ */
+ default int hashcode(int sourceIndex, T event) {
Review Comment:
Hi @loserwang1024, thanks for the PR! One suggestion on the `HashFunction`
change: instead of a raw `int sourceIndex`, consider passing a small immutable
`HashContext` object:
```java
@Internal
public class HashContext {
private final int sourceSubtaskIndex;
private final int downstreamParallelism;
public HashContext(int sourceSubtaskIndex, int downstreamParallelism) {
... }
public int getSourceSubtaskIndex() { ... }
public int getDownstreamParallelism() { ... }
}
```
```java
default int hashcode(HashContext context, T event) {
return hashcode(event);
}
```
Reasons:
1. **Self-documenting call sites** — `hashcode(subTaskId, event)` gives no
hint what the bare `int` means; `hashcode(hashContext, event)` does.
2. **Extensible without signature churn** — future strategies (bucket-aware
mapping, fallback when `sourceIndex >= downstreamParallelism`) can read from
the context instead of stacking more `int` overloads.
3. **Fails fast instead of silently skewing** —
`ForwardHashFunction.hashcode(event)` can throw `UnsupportedOperationException`
rather than returning `0` and routing everything to subtask 0 if someone calls
the single-arg method by mistake.
The context can be created once in `open()` and reused, so there's no
per-record allocation cost. Since `HashFunction` is `@Internal`, this is the
cheapest time to make the change. WDYT?
--
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]
