BewareMyPower commented on code in PR #18392:
URL: https://github.com/apache/pulsar/pull/18392#discussion_r1022961933
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java:
##########
@@ -66,8 +67,8 @@ public class PersistentAcknowledgmentsGroupingTracker
implements Acknowledgments
private final long acknowledgementGroupTimeMicros;
- private volatile TimedCompletableFuture<Void> currentIndividualAckFuture;
- private volatile TimedCompletableFuture<Void> currentCumulativeAckFuture;
+ private volatile CompletableFuture<Void> currentIndividualAckFuture;
+ private volatile CompletableFuture<Void> currentCumulativeAckFuture;
Review Comment:
Though a null completed `TimedCompletableFuture` might not make a difference
with `CompletableFuture.completedFuture(null)`, it still actually changed the
behavior a little that `Consumer#acknowledgeAsync` will return a
`TimedCompletableFuture` instead of `CompletableFuture`.
Someone could write (though it's only theoretically)
```java
var future = consumer.acknowledgeAsync(msgId);
if (future instanceof CompletableFuture) {
// AckReceipt is disabled
} else {
var timedFuture = (TimedCompletableFuture) future;
// AckReceipt is enabled
}
```
The implementations are also a little different, see my comments:
- https://github.com/apache/pulsar/pull/18392#discussion_r1021038684
- https://github.com/apache/pulsar/pull/18392#issuecomment-1310219078
> But it's a little different from CompletableFuture#completedFuture, which
only calls the constructor with the value.
--
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]
