From 71568ffee44a9bd1ad7c843dc06c996741c53837 Mon Sep 17 00:00:00 2001 From: xiaoxmeng Date: Fri, 10 Nov 2023 15:43:31 -0800 Subject: [PATCH] Add cache error message for all memory pool allocations (#7499) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/7499 Reviewed By: oerling Differential Revision: D51167510 Pulled By: xiaoxmeng fbshipit-source-id: a2b7893db77ce89e36e40e8b5fc5d181b17b64d8 --- velox/common/caching/AsyncDataCache.cpp | 4 +- velox/common/memory/MemoryPool.cpp | 15 +++-- velox/common/memory/tests/MemoryPoolTest.cpp | 66 ++++++++++++++------ 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/velox/common/caching/AsyncDataCache.cpp b/velox/common/caching/AsyncDataCache.cpp index b79af5ec30f2..3786a6f35623 100644 --- a/velox/common/caching/AsyncDataCache.cpp +++ b/velox/common/caching/AsyncDataCache.cpp @@ -671,8 +671,8 @@ bool AsyncDataCache::makeSpace( sizeMultiplier *= 2; } } - memory::setCacheFailureMessage(fmt::format( - "After failing to evict from cache state: {}", toString(false))); + memory::setCacheFailureMessage( + fmt::format("Failed to evict from cache state: {}", toString(false))); return false; } diff --git a/velox/common/memory/MemoryPool.cpp b/velox/common/memory/MemoryPool.cpp index fbd3b288d402..6d3634f31010 100644 --- a/velox/common/memory/MemoryPool.cpp +++ b/velox/common/memory/MemoryPool.cpp @@ -430,10 +430,11 @@ void* MemoryPoolImpl::allocate(int64_t size) { if (FOLLY_UNLIKELY(buffer == nullptr)) { release(alignedSize); VELOX_MEM_ALLOC_ERROR(fmt::format( - "{} failed with {} from {}", + "{} failed with {} from {} {}", __FUNCTION__, succinctBytes(size), - toString())); + toString(), + allocator_->getAndClearFailureMessage())); } DEBUG_RECORD_ALLOC(buffer, size); return buffer; @@ -448,11 +449,12 @@ void* MemoryPoolImpl::allocateZeroFilled(int64_t numEntries, int64_t sizeEach) { if (FOLLY_UNLIKELY(buffer == nullptr)) { release(alignedSize); VELOX_MEM_ALLOC_ERROR(fmt::format( - "{} failed with {} entries and {} each from {}", + "{} failed with {} entries and {} each from {} {}", __FUNCTION__, numEntries, succinctBytes(sizeEach), - toString())); + toString(), + allocator_->getAndClearFailureMessage())); } DEBUG_RECORD_ALLOC(buffer, size); return buffer; @@ -467,11 +469,12 @@ void* MemoryPoolImpl::reallocate(void* p, int64_t size, int64_t newSize) { if (FOLLY_UNLIKELY(newP == nullptr)) { release(alignedNewSize); VELOX_MEM_ALLOC_ERROR(fmt::format( - "{} failed with new {} and old {} from {}", + "{} failed with new {} and old {} from {} {}", __FUNCTION__, succinctBytes(newSize), succinctBytes(size), - toString())); + toString(), + allocator_->getAndClearFailureMessage())); } DEBUG_RECORD_ALLOC(newP, newSize); if (p != nullptr) { diff --git a/velox/common/memory/tests/MemoryPoolTest.cpp b/velox/common/memory/tests/MemoryPoolTest.cpp index ecf75b5683b3..af80176bb316 100644 --- a/velox/common/memory/tests/MemoryPoolTest.cpp +++ b/velox/common/memory/tests/MemoryPoolTest.cpp @@ -588,27 +588,53 @@ TEST_P(MemoryPoolTest, MemoryCapExceptions) { ASSERT_EQ(error_code::kMemAllocError.c_str(), ex.errorCode()); ASSERT_TRUE(ex.isRetriable()); if (useMmap_) { - ASSERT_EQ( - fmt::format( - "allocate failed with 128.00MB from Memory Pool[" - "static_quota LEAF root[MemoryCapExceptions] " - "parent[MemoryCapExceptions] MMAP track-usage {}]", - isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), - ex.message()); + if (useCache_) { + ASSERT_EQ( + fmt::format( + "allocate failed with 128.00MB from Memory Pool[" + "static_quota LEAF root[MemoryCapExceptions] " + "parent[MemoryCapExceptions] MMAP track-usage {}] Failed to evict from cache state: AsyncDataCache:\nCache size: 0B tinySize: 0B large size: 0B\nCache entries: 0 read pins: 0 write pins: 0 pinned shared: 0B pinned exclusive: 0B\n num write wait: 0 empty entries: 0\nCache access miss: 0 hit: 0 hit bytes: 0B eviction: 0 eviction checks: 0\nPrefetch entries: 0 bytes: 0B\nAlloc Megaclocks 0\nAllocated pages: 0 cached pages: 0\n", + isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), + ex.message()); + } else { + ASSERT_EQ( + fmt::format( + "allocate failed with 128.00MB from Memory Pool[" + "static_quota LEAF root[MemoryCapExceptions] " + "parent[MemoryCapExceptions] MMAP track-usage {}] ", + isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), + ex.message()); + } } else { - ASSERT_EQ( - fmt::format( - "allocate failed with 128.00MB from Memory Pool" - "[static_quota LEAF root[MemoryCapExceptions] " - "parent[MemoryCapExceptions] MALLOC track-usage {}]" - "", - isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), - ex.message()); + if (useCache_) { + ASSERT_EQ( + fmt::format( + "allocate failed with 128.00MB from Memory Pool" + "[static_quota LEAF root[MemoryCapExceptions] " + "parent[MemoryCapExceptions] MALLOC track-usage {}]" + " Failed to evict from cache state: AsyncDataCache:\nCache size: 0B tinySize: 0B large size: 0B\nCache entries: 0 read pins: 0 write pins: 0 pinned shared: 0B pinned exclusive: 0B\n num write wait: 0 empty entries: 0\nCache access miss: 0 hit: 0 hit bytes: 0B eviction: 0 eviction checks: 0\nPrefetch entries: 0 bytes: 0B\nAlloc Megaclocks 0\nAllocated pages: 0 cached pages: 0\n", + isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), + ex.message()); + } else { + ASSERT_EQ( + fmt::format( + "allocate failed with 128.00MB from Memory Pool" + "[static_quota LEAF root[MemoryCapExceptions] " + "parent[MemoryCapExceptions] MALLOC track-usage {}]" + " ", + isLeafThreadSafe_ ? "thread-safe" : "non-thread-safe"), + ex.message()); + } } } }