m1a2st commented on code in PR #19862: URL: https://github.com/apache/kafka/pull/19862#discussion_r2117798086
########## 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: In the `safeTryCompleteOrElse()` method, `tryComplete()` is called without first checking `isCompleted()`. we could move the `isCompleted()` check into `safeTryCompleteOrElse()` and remove in `forceComplete()`. -- 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