This is an automated email from the ASF dual-hosted git repository.
arshad pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 91e0520 ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush
on a stable internal value
91e0520 is described below
commit 91e0520133b82acb87ab60962fce5eae992d87e8
Author: Sylvain Wallez <[email protected]>
AuthorDate: Wed Mar 30 23:54:55 2022 +0530
ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable
internal value
When packets are added to ClientCnxn's outgoing packet queue we ensure
there's no conflict with an ongoing flush of that queue because of connection
loss.
Synchronization used to be on the state field's value. This value is both
not stable (its value changes over time), possibly causing improper
synchronization, and global, which can cause contention in applications that
run several ZooKeeper clients.
We now synchronize on outgoingQueue which is both local to a ClientCnxn's
instance and stable.
Author: Sylvain Wallez <[email protected]>
Reviewers: maoling <[email protected]>, Mohammad Arshad <[email protected]>
Closes #1257 from swallez/ZOOKEEPER-3652 and squashes the following commits:
82e2cad2c [Sylvain Wallez] Instruct SpotBugs that we know what we're doing
when synchronizing on outgoingQueue
b0bc03d6f [Sylvain Wallez] ZOOKEEPER-3652: Synchronize ClientCnxn outgoing
queue flush on a stable internal value
---
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
index 347a68f..cef1743 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
@@ -1175,6 +1175,7 @@ public class ClientCnxn {
}
@Override
+ @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
public void run() {
clientCnxnSocket.introduce(this, sessionId, outgoingQueue);
clientCnxnSocket.updateNow();
@@ -1303,7 +1304,7 @@ public class ClientCnxn {
}
}
- synchronized (state) {
+ synchronized (outgoingQueue) {
// When it comes to this point, it guarantees that later queued
// packet to outgoingQueue will be notified of death.
cleanup();
@@ -1645,6 +1646,7 @@ public class ClientCnxn {
return queuePacket(h, r, request, response, cb, clientPath,
serverPath, ctx, watchRegistration, null);
}
+ @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
public Packet queuePacket(
RequestHeader h,
ReplyHeader r,
@@ -1671,7 +1673,7 @@ public class ClientCnxn {
// 1. synchronize with the final cleanup() in SendThread.run() to
avoid race
// 2. synchronized against each packet. So if a closeSession packet is
added,
// later packet will be notified.
- synchronized (state) {
+ synchronized (outgoingQueue) {
if (!state.isAlive() || closing) {
conLossPacket(packet);
} else {
