[GitHub] [pulsar] lhotari commented on a change in pull request #10280: Fix thread safety issue in ManagedLedgerImpl

[GitBox <gi...@apache.org>]

lhotari commented on a change in pull request #10280:
URL: https://github.com/apache/pulsar/pull/10280#discussion_r616823604



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -195,11 +195,11 @@
     private final static CompletableFuture<PositionImpl> NULL_OFFLOAD_PROMISE = CompletableFuture
             .completedFuture(PositionImpl.latest);
     private volatile LedgerHandle currentLedger;
-    private long currentLedgerEntries = 0;
-    private long currentLedgerSize = 0;
-    private long lastLedgerCreatedTimestamp = 0;
-    private long lastLedgerCreationFailureTimestamp = 0;
-    private long lastLedgerCreationInitiationTimestamp = 0;
+    private volatile long currentLedgerEntries = 0;

Review comment:
       That's true for updating, but reading doesn't seem to hold the mutex in all cases? That could lead to "word tearing" problem at read time on 32-bit JVMs? As mentioned in the other comments, this isn't an issue on 64-bit JVMs. Perhaps running the Broker on non-64 bit JVMs should simply be unsupported and possibly prevented at startup?




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