Skip to content

Commit 4af5e24

Browse files
Jaesoo Leefacebook-github-bot
authored andcommitted
fix flaky unit tests in NvmCacheTest
Summary: This change fixes flaky unit tests in NvmCacheTest. For NvmCacheTest.IsNewCacheInstanceStat, previously, we were checking if nvm cache is new (GlobalCacheStats.isNewNvmCache) by comparing cache instance creation time and the nvm cache's creation time https://fburl.com/code/9x4fcl34. However, since we are resetting the nvm cache's creation time to current time when marking truncate (https://fburl.com/code/xzsvozpj), this is timing depenedt and thus could get wrong. This was causing NvmCacheTest.IsNewCacheInstanceStat (https://fburl.com/code/2sr8eonn) to fail often (e.g., https://fburl.com/testinfra/9045ppjx) For NvmCacheTest.testCreateItemAsIOBufChained, the creation time of chained item is reset when it is created as iobuf, so the verification check against the creation time of the item is wrong. Reviewed By: therealgymmy Differential Revision: D49324117 fbshipit-source-id: 4b85c6902fb663724be8d6e9eebb75d78109768b
1 parent a1a91c2 commit 4af5e24

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3412,8 +3412,10 @@ GlobalCacheStats CacheAllocator<CacheTrait>::getGlobalCacheStats() const {
34123412
ret.numActiveHandles = getNumActiveHandles();
34133413

34143414
ret.isNewRamCache = cacheCreationTime_ == cacheInstanceCreationTime_;
3415+
// NVM cache is new either if newly created or started fresh with truncate
34153416
ret.isNewNvmCache =
3416-
nvmCacheState_.getCreationTime() == cacheInstanceCreationTime_;
3417+
(nvmCacheState_.getCreationTime() == cacheInstanceCreationTime_) ||
3418+
nvmCacheState_.shouldStartFresh();
34173419

34183420
return ret;
34193421
}

cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,8 +2179,10 @@ TEST_F(NvmCacheTest, testEvictCB) {
21792179
void verifyItem(const Item& item, const Item& iobufItem) {
21802180
ASSERT_EQ(item.isChainedItem(), iobufItem.isChainedItem());
21812181
ASSERT_EQ(item.hasChainedItem(), iobufItem.hasChainedItem());
2182-
ASSERT_EQ(item.getCreationTime(), iobufItem.getCreationTime());
2183-
ASSERT_EQ(item.getExpiryTime(), iobufItem.getExpiryTime());
2182+
if (!item.isChainedItem()) {
2183+
ASSERT_EQ(item.getCreationTime(), iobufItem.getCreationTime());
2184+
ASSERT_EQ(item.getExpiryTime(), iobufItem.getExpiryTime());
2185+
}
21842186
ASSERT_EQ(item.getSize(), iobufItem.getSize());
21852187
ASSERT_EQ(
21862188
0, std::memcmp(item.getMemory(), iobufItem.getMemory(), item.getSize()));

0 commit comments

Comments
 (0)