KAFKA-18694: Migrate suitable classes to recorods in coordinator-common module #18782
Conversation
| private record TaskResult<R>(R result, Throwable exception) { | ||
| } |
There was a problem hiding this comment.
Could we merge them into one line?
| private record TaskResult<R>(R result, Throwable exception) { | |
| } | |
| private record TaskResult<R>(R result, Throwable exception) { } |
| record LoadSummary(long startTimeMs, long endTimeMs, long schedulerQueueTimeMs, long numRecords, long numBytes) { | ||
| } |
| private record Timestamped<T>(long timestamp, T value) { | ||
| } |
Thanks for the comments.Fixed. |
| @Override | ||
| public String toString() { | ||
| return "LoadSummary(" + | ||
| "startTimeMs=" + startTimeMs + | ||
| ", endTimeMs=" + endTimeMs + | ||
| ", schedulerQueueTimeMs=" + schedulerQueueTimeMs + | ||
| ", numRecords=" + numRecords + | ||
| ", numBytes=" + numBytes + ")"; | ||
| } |
There was a problem hiding this comment.
Record the default toString method will show "Square brackets" instead "Parentheses", ie. LoadSummary(...) will change to LoadSummary[...], If won't broken any test, I think it will be OK.
|
How are you evaluating if these classes can be safely converted? There are clear differences in behavior in equals/hashCode after these changes. |
Hi @ijuma Thanks for catching this issue.You make a good point about the different behaviors in equals/hashCode. While the Java SE docs mention these differences:
in our implementation we're actually comparing the object fields directly rather than using equals() for object comparison.E.g: And we don't invoke hashCode() directly either. Based on this approach, I believe these conversions should maintain compatibility without issues. |
jira: https://issues.apache.org/jira/browse/KAFKA-18694
As title.
Committer Checklist (excluded from commit message)