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]