pulsar-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] lovelle commented on a change in pull request #3581: Fix thread safety violation on ProducerImpl
Date Thu, 14 Feb 2019 03:19:41 GMT
lovelle commented on a change in pull request #3581: Fix thread safety violation on ProducerImpl
URL: https://github.com/apache/pulsar/pull/3581#discussion_r256678144
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
 ##########
 @@ -803,12 +806,12 @@ protected boolean verifyLocalBufferIsNotCorrupted(OpSendMsg op) {
     }
 
     protected static final class OpSendMsg {
-        MessageImpl<?> msg;
-        List<MessageImpl<?>> msgs;
-        ByteBufPair cmd;
-        SendCallback callback;
-        long sequenceId;
-        long createdAt;
+        volatile MessageImpl<?> msg;
 
 Review comment:
   > Is it too heavy to convert all fields to `volatile`?
   
   In a general sense no, it should be as cheap as load memory from L1 cache on avg, however
if another thread on another core is writing the volatile field the cache-line will be invalidated
and would require to load memory access from different cache access, of course this behavior
depends heavily on cpu arch. Since this is done on client side and is safer than a raw variable,
the overhead of this is I think is negligible.
   
   > Have you seen any issues regarding these fields?
   
   Nope, anyway, I will investigate this further and will try to add a test to reproduce this,
but since it depends on very unlucky timing it might be difficult to reproduce.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Mime
View raw message