Skip to content

Commit

Permalink
Update comment and improve error reporting
Browse files Browse the repository at this point in the history
Signed-off-by: Gavin Halliday <[email protected]>
  • Loading branch information
ghalliday committed Apr 2, 2024
1 parent e835a42 commit 45735ad
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions system/jhtree/jhtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ class CNodeMapping : public HTMapping<CNodeCacheEntry, CKeyIdAndPos>
};

typedef OwningSimpleHashTableOf<CNodeMapping, CKeyIdAndPos> CNodeTable;
class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMapping, CNodeTable>
class CNodeMRUCache final : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMapping, CNodeTable>
{
std::atomic<size32_t> sizeInMem{0};
size32_t memLimit = 0;
Expand All @@ -706,10 +706,13 @@ class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMap
virtual void makeSpace()
{
// remove LRU until !full
// This code could walk the list, rather than restarting at the end each time - but there are unlikely to be
// many entries that have no associated node, and nodes could have been associated in the meantime.
do
{
CNodeMapping *tail = mruList.tail();
assertex(tail);
if (unlikely(!tail))
throw makeStringExceptionV(9999, "Index cache appears full but contains no entries size=%x limit=%x", sizeInMem, memLimit);

//Never evict an entry that hasn't yet loaded - otherwise the sizeInMem can become inconsistent
//When running with slow remote storage this can take a long time to be ready - so we need
Expand All @@ -719,7 +722,11 @@ class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMap
{
tail = tail->prev;
if (!tail)
return; // no pages in the cache are ready - this should never happen because full() should be false
{
// no pages in the cache are ready - this could possibly happen in a tiny race-window where
// sizes in the cache have been updated, but no nodes have yet been associated with the entries.
return;
}
}

mruList.remove(tail);
Expand Down

0 comments on commit 45735ad

Please sign in to comment.