jsancio commented on code in PR #16563:
URL: https://github.com/apache/kafka/pull/16563#discussion_r1674382183
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientTest.java:
##########
@@ -281,15 +281,20 @@ public void testGrantVotesWhenShuttingDown(boolean
withKip853Rpc) throws Excepti
context.client.poll();
// We will first transition to unattached and then grant vote and then
transition to voted
- assertTrue(context.client.quorum().isVoted());
+ assertTrue(
+ context.client.quorum().isVoted(),
+ "Local Id: " + localId +
+ " Remote Id: " + remoteId +
+ " Quorum local Id: " +
context.client.quorum().localIdOrSentinel()
+ + " Quorum leader Id: " +
context.client.quorum().leaderIdOrSentinel());
Review Comment:
How about:
```java
"Local Id: " + localId +
" Remote Id: " + remoteId +
" Quorum local Id: " +
context.client.quorum().localIdOrSentinel() +
" Quorum leader Id: " +
context.client.quorum().leaderIdOrSentinel()
);
```
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientSnapshotTest.java:
##########
@@ -1977,12 +1978,20 @@ private static ReplicaKey replicaKey(int id, boolean
withDirectoryId) {
return ReplicaKey.of(id, directoryId);
}
+ private static Integer randomReplicaId() {
+ int mockAddressPrefix = 9990;
+ // Number of nodes we can set up if we got a random number that is
maximum in the range
+ int reservedNumberOfPorts = 50;
+ int maxPort = 65535 - mockAddressPrefix - reservedNumberOfPorts;
+ return ThreadLocalRandom.current().nextInt(maxPort);
+ }
Review Comment:
Let's return an `int` instead of the object `Integer`: `private static int
randomReplicaId()`.
The variable names `mockAddressPrefix` and `reserverdNumberOfPorts` are
misleading this id has nothing to do with those concepts. If you want to limit
the range of the integer, I am okay with generating a random id from 0 to 1024.
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientTest.java:
##########
@@ -3860,12 +3865,11 @@ private static ReplicaKey replicaKey(int id, boolean
withDirectoryId) {
return ReplicaKey.of(id, directoryId);
}
- private static Integer getRandomPort() {
- int minPort = 1024;
+ private static Integer randomReplicaId() {
int mockAddressPrefix = 9990;
// Number of nodes we can set up if we got a random number that is
maximum in the range
int reservedNumberOfPorts = 50;
int maxPort = 65535 - mockAddressPrefix - reservedNumberOfPorts;
Review Comment:
Let's return an `int` instead of the object `Integer`: `private static int
randomReplicaId()`.
The variable names `mockAddressPrefix` and `reserverdNumberOfPorts` are
misleading this id has nothing to do with those concepts. If you want to limit
the range of the integer, I am okay with generating a random id from 0 to 1024.
--
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]