m1a2st commented on code in PR #19862:
URL: https://github.com/apache/kafka/pull/19862#discussion_r2121355928
##########
server-common/src/main/java/org/apache/kafka/server/purgatory/DelayedOperation.java:
##########
@@ -51,32 +51,15 @@ public DelayedOperation(long delayMs) {
*
* 1. The operation has been verified to be completable inside
tryComplete()
* 2. The operation has expired and hence needs to be completed right now
- *
- * Return true iff the operation is completed by the caller: note that
- * concurrent threads can try to complete the same operation, but only
- * the first thread will succeed in completing the operation and return
- * true, others will still return false.
+ *
*/
- public boolean forceComplete() {
+ public void forceComplete() {
// Do not proceed if the operation is already completed.
- if (completed) {
- return false;
- }
- // Attain lock prior completing the request.
- lock.lock();
- try {
- // Re-check, if the operation is already completed by some other
thread.
- if (!completed) {
- completed = true;
- // cancel the timeout timer
- cancel();
- onComplete();
- return true;
- } else {
- return false;
- }
- } finally {
- lock.unlock();
+ if (!completed) {
Review Comment:
> Removing the condition introduces another restriction: forceComplete must
be called only once in tryComplete. That may be error-prone I think
To ensure that `forceComplete()` is called at most once inside
`tryComplete()`, we would have to assume that all logic inside
`forceComplete()` will not throw any exception.
I don’t think we can safely assume this, since `onComplete()` is implemented
by subclasses. So it’s safer to keep the completion check inside
`forceComplete()`.
The `cancel()` and `onComplete()` methods will not be executed a second time
when `forceComplete()` is called inside the catch block.
--
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
