From ec5323810b8bfe6c3e63d00810a685d9da83c52f Mon Sep 17 00:00:00 2001 From: Rutvij Mehta Date: Fri, 9 Feb 2024 01:34:36 -0800 Subject: [PATCH] MINOR: Fix PartitionRegistration.hashCode (#12774) (#77) `PartitionRegistration.hashCode` passes raw arrays to `Objects.hash` in the `hashCode` implementation. This doesn't work since the `equals` implementation uses `Arrays.equals`. We should use `Arrays.hashCode` instead. Reviewers: David Arthur Co-authored-by: Jason Gustafson --- .../kafka/metadata/PartitionRegistration.java | 4 +- .../metadata/PartitionRegistrationTest.java | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java b/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java index 89ca9648eaf7..c97ad4352cf1 100644 --- a/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java +++ b/metadata/src/main/java/org/apache/kafka/metadata/PartitionRegistration.java @@ -210,8 +210,8 @@ public boolean isReassigning() { @Override public int hashCode() { - return Objects.hash(replicas, isr, removingReplicas, addingReplicas, leader, leaderRecoveryState, - leaderEpoch, partitionEpoch); + return Objects.hash(Arrays.hashCode(replicas), Arrays.hashCode(isr), Arrays.hashCode(removingReplicas), + Arrays.hashCode(addingReplicas), leader, leaderRecoveryState, leaderEpoch, partitionEpoch); } @Override diff --git a/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java b/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java index 66bf3fe03333..fa19149a3f56 100644 --- a/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java +++ b/metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java @@ -17,6 +17,11 @@ package org.apache.kafka.metadata; +import net.jqwik.api.Arbitraries; +import net.jqwik.api.Arbitrary; +import net.jqwik.api.ForAll; +import net.jqwik.api.Property; +import net.jqwik.api.Provide; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.Uuid; import org.apache.kafka.common.message.LeaderAndIsrRequestData.LeaderAndIsrPartitionState; @@ -31,6 +36,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -125,4 +131,43 @@ public void testMergePartitionChangeRecordWithReassignmentData() { new int[] {1, 2, 4}, Replicas.NONE, Replicas.NONE, 1, LeaderRecoveryState.RECOVERED, 100, 202), partition2); assertFalse(partition2.isReassigning()); } + + @Property + public void testConsistentEqualsAndHashCode( + @ForAll("uniqueSamples") PartitionRegistration a, + @ForAll("uniqueSamples") PartitionRegistration b + ) { + if (a.equals(b)) { + assertEquals(a.hashCode(), b.hashCode()); + } + + if (a.hashCode() != b.hashCode()) { + assertNotEquals(a, b); + } + } + + @Provide + Arbitrary uniqueSamples() { + return Arbitraries.of( + new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE, + 1, LeaderRecoveryState.RECOVERED, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE, + 1, LeaderRecoveryState.RECOVERED, 101, 200), + new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE, + 1, LeaderRecoveryState.RECOVERED, 100, 201), + new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, Replicas.NONE, + 2, LeaderRecoveryState.RECOVERED, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3}, new int[] {1}, Replicas.NONE, Replicas.NONE, + 1, LeaderRecoveryState.RECOVERING, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {4, 5, 6}, new int[] {1, 2, 3}, + 1, LeaderRecoveryState.RECOVERED, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {1, 2, 3}, new int[] {4, 5, 6}, + 1, LeaderRecoveryState.RECOVERED, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, new int[] {1, 2, 3}, Replicas.NONE, + 1, LeaderRecoveryState.RECOVERED, 100, 200), + new PartitionRegistration(new int[] {1, 2, 3, 4, 5, 6}, new int[] {1, 2, 3}, Replicas.NONE, new int[] {4, 5, 6}, + 1, LeaderRecoveryState.RECOVERED, 100, 200) + ); + } + }