gortiz commented on code in PR #14847:
URL: https://github.com/apache/pinot/pull/14847#discussion_r1927115243
##########
pinot-common/src/main/java/org/apache/pinot/common/concurrency/AdjustableSemaphore.java:
##########
@@ -20,32 +20,44 @@
import com.google.common.base.Preconditions;
import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* A semaphore that allows adjusting the number of permits in a non-blocking
way.
*/
public class AdjustableSemaphore extends Semaphore {
- private int _totalPermits;
+ private final AtomicInteger _totalPermits;
public AdjustableSemaphore(int permits) {
super(permits);
- _totalPermits = permits;
+ _totalPermits = new AtomicInteger(permits);
}
public AdjustableSemaphore(int permits, boolean fair) {
super(permits, fair);
- _totalPermits = permits;
+ _totalPermits = new AtomicInteger(permits);
}
+ /**
+ * Sets the total number of permits to the given value without blocking.
+ */
public void setPermits(int permits) {
Preconditions.checkArgument(permits > 0, "Permits must be a positive
integer");
- if (permits < _totalPermits) {
- reducePermits(_totalPermits - permits);
- } else if (permits > _totalPermits) {
- release(permits - _totalPermits);
+ if (permits < _totalPermits.get()) {
+ reducePermits(_totalPermits.get() - permits);
+ } else if (permits > _totalPermits.get()) {
+ release(permits - _totalPermits.get());
Review Comment:
That is what I thought. Volatile should be enough to be correct. In
practice, you could even remove the volatile as you suggest, but in theory,
that could imply that the reader threads never read the new value. Also, IIRC,
in x86, to read a volatile attribute that was not modified is exactly as to
read a nonvolatile attribute. The only actual issue is if that attribute ends
up creating false sharing with another, frequently written, not volatile
attribute
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
