You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2022/04/05 06:44:04 UTC
[GitHub] [zookeeper] symat opened a new pull request, #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
symat opened a new pull request, #1850:
URL: https://github.com/apache/zookeeper/pull/1850
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 <sy...@bluxte.net>
Reviewers: maoling <ma...@apache.org>, Mohammad Arshad <ar...@apache.org>
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
(cherry picked from commit 91e0520133b82acb87ab60962fce5eae992d87e8)
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] symat commented on pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
symat commented on PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#issuecomment-1089851366
hello @arshadmohammad and @maoling !
you already reviewed this fix on newer branches. Can you also review this one? (we were facing thread leak issues in NiFI with ZooKeeper client 3.5.5 and we think this commit might cause the problem. I want this fix to be released in the next 3.5 version.)
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] symat commented on pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
symat commented on PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#issuecomment-1090124423
thanks for the quick review - merge!
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#issuecomment-1089856365
Changes look good to me but there are many formatting related change. Please revert all formatting changes
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad closed pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
arshadmohammad closed pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
URL: https://github.com/apache/zookeeper/pull/1850
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#issuecomment-1089948669
merged. Thanks @symat for raising the PR
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on a diff in pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on code in PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#discussion_r843497422
##########
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java:
##########
@@ -178,7 +178,7 @@
* operation)
*/
private volatile boolean closing = false;
-
+
Review Comment:
Please revert formatting changes.
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] symat commented on pull request #1850: ZOOKEEPER-3652: Synchronize ClientCnxn outgoing queue flush on a stable internal value
Posted by GitBox <gi...@apache.org>.
symat commented on PR #1850:
URL: https://github.com/apache/zookeeper/pull/1850#issuecomment-1089858016
true, sorry (blaming it on IntelliJ :) ) - will fix it soon
--
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: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org