apoorvmittal10 commented on code in PR #19862:
URL: https://github.com/apache/kafka/pull/19862#discussion_r2124860736
##########
jmh-benchmarks/src/main/java/org/apache/kafka/jmh/core/TestPurgatoryPerformance.java:
##########
@@ -479,7 +479,8 @@ public void onComplete() {
@Override
public boolean tryComplete() {
- return System.currentTimeMillis() >= completesAt &&
forceComplete();
+ forceComplete();
+ return System.currentTimeMillis() >= completesAt && isCompleted();
Review Comment:
Shouldn't this be as below?
```suggestion
if (System.currentTimeMillis() >= completesAt) {
forceComplete();
return true;
}
return false;
```
##########
server/src/main/java/org/apache/kafka/server/purgatory/DelayedDeleteRecords.java:
##########
@@ -91,7 +91,13 @@ public boolean tryComplete() {
}
});
// check if every partition has satisfied at least one of case A or B
- return
deleteRecordsStatus.values().stream().noneMatch(DeleteRecordsPartitionStatus::acksPending)
&& forceComplete();
+ boolean noPendingPartition = deleteRecordsStatus.values()
+ .stream()
+ .noneMatch(DeleteRecordsPartitionStatus::acksPending);
+ if (noPendingPartition) {
+ forceComplete();
+ }
+ return noPendingPartition && isCompleted();
Review Comment:
if `noPendingPartition` is true then `forceComplete` will be invoked which
should always return `isCompleted()` as true, correct? Hence the code shouldn't
be like below?
```suggestion
if (noPendingPartition) {
forceComplete();
return true;
}
return false;
```
--
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
