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 2021/04/24 05:13:36 UTC

[GitHub] [zookeeper] Technoboy- opened a new pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Technoboy- opened a new pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-660449975


   +0 for this. I wonder whether `finished` can be visited by multiply threads for the usage of visiblity of `volatile`. Of  course, it's always harmless.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-826256256


   @Technoboy-       Thanks for your contribution.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Technoboy- commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-658502444


   Could someone take a look at the checks ?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-661739791


   Thanks @Technoboy- for the improvement!
   
   Actually the variable is accessed by multiple threads (e.g. the `run()` vs. the `shutdown()` methods). So adding the `volatile` would make sense. Usually I prefer to use the Atomic* objects more. But maybe for a simple boolean set/check the volatile is enough.
   
   But I don't really like these micro-commits, it's just too much administration :)
   Maybe @Technoboy- could you also check the other processors and look if you find other places where you can improve the shutdown mechanisms? Of course only if you have the time...


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] asfgit closed pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling closed pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
maoling closed pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] asfgit closed pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] Technoboy- commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-662787740


   > mechanisms
   
   Thanks @symat . These small changes are really too much administration. But it's the interactive as learning the codings.
   I will and would like to try something more challenge.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling commented on pull request #1402: ZOOKEEPER-3889: Add volatile for variable finished in ReadOnlyRequestProcessor

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1402:
URL: https://github.com/apache/zookeeper/pull/1402#issuecomment-826256256


   @Technoboy-       Thanks for your contribution.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org