bowenliang123 commented on code in PR #5217:
URL: https://github.com/apache/kyuubi/pull/5217#discussion_r1309590859
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/SessionLimiter.scala:
##########
@@ -95,7 +95,10 @@ class SessionLimiterImpl(userLimit: Int, ipAddressLimit:
Int, userIpAddressLimit
private def decrLimitCount(key: String): Unit = {
_counters.get(key) match {
- case count: AtomicInteger => count.decrementAndGet()
+ case count: AtomicInteger =>
+ if (count.decrementAndGet() < 0) {
+ count.incrementAndGet()
+ }
Review Comment:
```suggestion
case count: AtomicInteger if count.get > 0 =>
count.synchronized {
val countValue = count.get
if (countValue > 0) {
count.compareAndSet(countValue, countValue - 1)
}
}
```
Back and forth operations on AtomicInteger will cause side effects, as no an
extra thread-safe measure applied here.
Consider using the CAS for changing the value of AtomicInteger. And even if
the CAS failed, it will be ignored and it won't cause side effects or
exceptions.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
