From 1703574d503c4cc0ef63da36be51b38bb9b7a2bc Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Wed, 20 Aug 2025 08:17:17 +0700 Subject: [PATCH] fix: add actual transaction data to RWDB memory usage calculations The existing memory usage functions only counted structural overhead (sizeof of containers and pointers) but missed the actual transaction and metadata blob sizes. This caused severe underreporting - showing KBs when actually using MBs or GBs. Changes: - Keep existing structural overhead calculations - Add actual transaction/metadata serialized data sizes - Use transactionMap_ as single source to avoid double-counting - Add MAP_NODE_OVERHEAD constant for red-black tree nodes (~40 bytes each) - Use vector::capacity() instead of size() for actual allocated memory - Include ledger's transaction map node overhead in ledger calculations - Change internal calculation to uint64_t to prevent overflow - Add clear comments explaining what each section measures These improvements provide much more accurate memory reporting for monitoring and diagnostic purposes. --- src/ripple/app/rdb/backend/RWDBDatabase.h | 119 ++++++++++++++++------ src/test/rdb/RelationalDatabase_test.cpp | 4 +- 2 files changed, 88 insertions(+), 35 deletions(-) diff --git a/src/ripple/app/rdb/backend/RWDBDatabase.h b/src/ripple/app/rdb/backend/RWDBDatabase.h index 70d0e4ee8..f91e50cda 100644 --- a/src/ripple/app/rdb/backend/RWDBDatabase.h +++ b/src/ripple/app/rdb/backend/RWDBDatabase.h @@ -445,55 +445,108 @@ public: return true; // In-memory database always has space } + // Red-black tree node overhead per map entry + static constexpr size_t MAP_NODE_OVERHEAD = 40; + +private: + std::uint64_t + getBytesUsedLedger_unlocked() const + { + std::uint64_t size = 0; + + // Count structural overhead of ledger storage including map node + // overhead Note: sizeof(LedgerData) includes the map container for + // transactions, but not the actual transaction data + size += ledgers_.size() * + (sizeof(LedgerIndex) + sizeof(LedgerData) + MAP_NODE_OVERHEAD); + + // Add the transaction map nodes inside each ledger (ledger's view of + // its transactions) + for (const auto& [_, ledgerData] : ledgers_) + { + size += ledgerData.transactions.size() * + (sizeof(uint256) + sizeof(AccountTx) + MAP_NODE_OVERHEAD); + } + + // Count the ledger hash to sequence lookup map + size += ledgerHashToSeq_.size() * + (sizeof(uint256) + sizeof(LedgerIndex) + MAP_NODE_OVERHEAD); + + return size; + } + + std::uint64_t + getBytesUsedTransaction_unlocked() const + { + if (!useTxTables_) + return 0; + + std::uint64_t size = 0; + + // Count structural overhead of transaction map + // sizeof(AccountTx) is just the size of two shared_ptrs (~32 bytes) + size += transactionMap_.size() * + (sizeof(uint256) + sizeof(AccountTx) + MAP_NODE_OVERHEAD); + + // Add actual transaction and metadata data sizes + for (const auto& [_, accountTx] : transactionMap_) + { + if (accountTx.first) + size += accountTx.first->getSTransaction() + ->getSerializer() + .peekData() + .size(); + if (accountTx.second) + size += accountTx.second->getAsObject() + .getSerializer() + .peekData() + .size(); + } + + // Count structural overhead of account transaction index + // The actual transaction data is already counted above from + // transactionMap_ + for (const auto& [accountId, accountData] : accountTxMap_) + { + size += + sizeof(accountId) + sizeof(AccountTxData) + MAP_NODE_OVERHEAD; + for (const auto& [ledgerSeq, txVector] : accountData.ledgerTxMap) + { + // Use capacity() to account for actual allocated memory + size += sizeof(ledgerSeq) + MAP_NODE_OVERHEAD; + size += txVector.capacity() * sizeof(AccountTx); + } + } + + return size; + } + +public: std::uint32_t getKBUsedAll() override { std::shared_lock lock(mutex_); - std::uint32_t size = sizeof(*this); - size += ledgers_.size() * (sizeof(LedgerIndex) + sizeof(LedgerData)); - size += - ledgerHashToSeq_.size() * (sizeof(uint256) + sizeof(LedgerIndex)); - size += transactionMap_.size() * (sizeof(uint256) + sizeof(AccountTx)); - for (const auto& [_, accountData] : accountTxMap_) - { - size += sizeof(AccountID) + sizeof(AccountTxData); - for (const auto& [_, txVector] : accountData.ledgerTxMap) - { - size += sizeof(uint32_t) + txVector.size() * sizeof(AccountTx); - } - } - return size / 1024; + + // Total = base object + ledger infrastructure + transaction data + std::uint64_t size = sizeof(*this) + getBytesUsedLedger_unlocked() + + getBytesUsedTransaction_unlocked(); + + return static_cast(size / 1024); } std::uint32_t getKBUsedLedger() override { std::shared_lock lock(mutex_); - std::uint32_t size = 0; - size += ledgers_.size() * (sizeof(LedgerIndex) + sizeof(LedgerData)); - size += - ledgerHashToSeq_.size() * (sizeof(uint256) + sizeof(LedgerIndex)); - return size / 1024; + return static_cast(getBytesUsedLedger_unlocked() / 1024); } std::uint32_t getKBUsedTransaction() override { - if (!useTxTables_) - return 0; - std::shared_lock lock(mutex_); - std::uint32_t size = 0; - size += transactionMap_.size() * (sizeof(uint256) + sizeof(AccountTx)); - for (const auto& [_, accountData] : accountTxMap_) - { - size += sizeof(AccountID) + sizeof(AccountTxData); - for (const auto& [_, txVector] : accountData.ledgerTxMap) - { - size += sizeof(uint32_t) + txVector.size() * sizeof(AccountTx); - } - } - return size / 1024; + return static_cast( + getBytesUsedTransaction_unlocked() / 1024); } void diff --git a/src/test/rdb/RelationalDatabase_test.cpp b/src/test/rdb/RelationalDatabase_test.cpp index 9eee7c6d1..12889247d 100644 --- a/src/test/rdb/RelationalDatabase_test.cpp +++ b/src/test/rdb/RelationalDatabase_test.cpp @@ -542,9 +542,9 @@ public: auto newLedgerKB = sqliteDb->getKBUsedLedger(); auto newTxKB = sqliteDb->getKBUsedTransaction(); - BEAST_EXPECT(newAllKB == 1); + BEAST_EXPECT(newAllKB == 2); BEAST_EXPECT(newLedgerKB == 0); - BEAST_EXPECT(newTxKB == 0); + BEAST_EXPECT(newTxKB == 1); // Test database closure operations (should not throw) try