junrao commented on code in PR #19759:
URL: https://github.com/apache/kafka/pull/19759#discussion_r2103085830
##########
server-common/src/main/java/org/apache/kafka/server/purgatory/DelayedOperation.java:
##########
@@ -68,24 +67,36 @@ public DelayedOperation(long delayMs, Lock lock) {
* 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
+ * true, others will still return false.
*/
public boolean forceComplete() {
- if (completed.compareAndSet(false, true)) {
- // cancel the timeout timer
- cancel();
- onComplete();
- return true;
- } else {
+ // Do not proceed if the operation is already completed.
Review Comment:
Only the expiration logic checks the return value of forceComplete(). We
could do the following in the expiration logic and change `forceComplete()` to
return void and avoid using the lock there (the caller will get the lock
instead).
```
lock.lock();
try {
if (!isCompleted()) {
forceComplete();
onExpiration();
}
} finally {
lock.unlock();
}
```
##########
server-common/src/main/java/org/apache/kafka/server/purgatory/DelayedOperation.java:
##########
@@ -41,7 +40,7 @@
*/
public abstract class DelayedOperation extends TimerTask {
- private final AtomicBoolean completed = new AtomicBoolean(false);
+ private volatile boolean completed = false;
Review Comment:
> onComplete() can be triggered by either forceComplete(), which forces
calling onComplete() after delayMs if the operation is not yet completed, or
tryComplete(), which first checks if the operation can be completed or not now,
and if yes calls forceComplete().
Could we adjust the comment to sth like the following?
onComplete() is called from forceComplete(), which is triggered once by
either expiration if the operation is not completed after delayMs, or
tryComplete() if the operation can be completed now.
--
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
