michaeljmarshall commented on code in PR #4307:
URL: https://github.com/apache/bookkeeper/pull/4307#discussion_r1681585838
##########
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:
Notice that we're aggregating the `writeCache` and `writeCacheBeingFlushed`
size and count. Given that the swap is just to replace references for
`writeCache` and `writeCacheBeingFlushed`, I am not sure I understand what is
wrong here. In this PR, we grab the references once and remove the possibility
of `writeCache.size() + writeCacheBeingFlushed.size()` double counting the same
value from `writeCache` in the event the caches are swapped after
`writeCache.size()` and before `writeCacheBeingFlushed.size()`
--
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]
