chia7712 commented on code in PR #15710:
URL: https://github.com/apache/kafka/pull/15710#discussion_r1562828732
##
server-common/src/test/java/org/apache/kafka/timeline/SnapshotRegistryTest.java:
##
@@ -41,9 +42,7 @@ public void testEmptyRegistry() {
private static void assertIteratorContains(Iterator iter,
Snapshot... snapshots) {
List expected = new ArrayList<>();
Review Comment:
How about `List expected = Arrays.asList(snapshots);`?
##
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsInstance.java:
##
@@ -116,8 +116,7 @@ public synchronized boolean
maybeUpdatePushRequestTimestamp(long currentTime) {
*/
boolean canAccept = lastGetRequestTimestamp > lastPushRequestTimestamp;
Review Comment:
how about `boolean canAccept = lastGetRequestTimestamp >
lastPushRequestTimestamp || currentTime - lastPushRequestTimestamp >=
pushIntervalMs;`?
##
server-common/src/test/java/org/apache/kafka/timeline/SnapshottableHashTableTest.java:
##
@@ -310,10 +309,8 @@ private static void assertIteratorYields(Iterator iter,
}
}
if (!extraObjects.isEmpty() || !remaining.isEmpty()) {
-throw new RuntimeException("Found extra object(s): [" +
String.join(", ",
-extraObjects.stream().map(e ->
e.toString()).collect(Collectors.toList())) +
-"] and didn't find object(s): [" + String.join(", ",
-remaining.keySet().stream().map(e ->
e.toString()).collect(Collectors.toList())) + "]");
+throw new RuntimeException("Found extra object(s): [" +
extraObjects.stream().map(e -> e.toString()).collect(Collectors.joining(", ")) +
Review Comment:
`e -> e.toString()` -> `Object::toString`
##
server/src/main/java/org/apache/kafka/server/AssignmentsManager.java:
##
@@ -321,7 +321,7 @@ public void run() throws Exception {
AssignReplicasToDirsResponseData data =
((AssignReplicasToDirsResponse) response.responseBody()).data();
Set failed = filterFailures(data, inflight);
-Set completed = Utils.diff(HashSet::new,
inflight.values().stream().collect(Collectors.toSet()), failed);
+Set completed = Utils.diff(HashSet::new, new
HashSet<>(inflight.values()), failed);
Review Comment:
Maybe we don't need to create collection many times by skipping the failed
elements.
```java
Set failed = filterFailures(data, inflight);
for (AssignmentEvent assignmentEvent : inflight.values()) {
if (failed.contains(assignmentEvent)) continue;
if (log.isDebugEnabled()) {
log.debug("Successfully propagated assignment {}",
assignmentEvent);
}
assignmentEvent.onComplete();
}
```
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
