liangyepianzhou commented on code in PR #4307:
URL: https://github.com/apache/bookkeeper/pull/4307#discussion_r1957544428


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java:
##########
@@ -207,10 +209,17 @@ public SingleDirectoryDbLedgerStorage(ServerConfiguration 
conf, LedgerManager le
         gcThread = new GarbageCollectorThread(conf,
                 ledgerManager, ledgerDirsManager, this, entryLogger, 
ledgerIndexDirStatsLogger);
 
+        // Because the writeCache and the writeCacheBeingFlushed are swapped 
during flushes, it is
+        // safer to acquire the underlying cache size and count references and 
then use those for metrics.
+        final AtomicLong finalWriteCacheSize = writeCache.cacheSize;
+        final AtomicLong finalWriteCacheBeingFlushedSize = 
writeCacheBeingFlushed.cacheSize;
+        final LongAdder finalWriteCacheCounter = writeCache.cacheCount;
+        final LongAdder finalWriteCacheBeingFlushedCounter = 
writeCacheBeingFlushed.cacheCount;
+

Review Comment:
   +1, I don't t think this is right. 
   
   The original logic used dynamic binding, where each lambda invocation 
dynamically retrieves the current instances of `writeCache` and 
`writeCacheBeingFlushed`. The modified approach statically binds by capturing 
the initial instance references of `writeCache` and `writeCacheBeingFlushed` in 
the constructor. This error severely distorts the storage monitoring data.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to