Skip to content

Commit 5e7ff9a

Browse files
Jimmy Lufacebook-github-bot
authored andcommitted
Fix flaky test - EvictToNvmGet
Summary: This test was flaky because sometimes a nvm put could be aborted due to an outstanding tombstone for the same key. This can happen because the test is inserting items very quickly and thus evicting quickly. The following scenario can happen: 1. insert "key1" and queue a remove operation for "key1" to NvmCache (via insertOrReplace) 2. insert "key2" ... "key100" 3. triggers eviction of "key1" while a remove tombstone is outstanding for "key1". This NvmCache put is thus aborted. The fix is to flush nvm-cache periodically to avoid such a race condition. Reviewed By: jaesoo-fb Differential Revision: D42443647 fbshipit-source-id: 56508b88b260b5a8fbe41b6d8dd5444e8f2c171f
1 parent f0f80c0 commit 5e7ff9a

File tree

1 file changed

+19
-1
lines changed

1 file changed

+19
-1
lines changed

cachelib/allocator/nvmcache/tests/NvmCacheTests.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,13 @@ TEST_F(NvmCacheTest, CouldExistFast) {
158158
}
159159

160160
TEST_F(NvmCacheTest, EvictToNvmGet) {
161+
// Disable bighash since we're only testing large items here
162+
this->config_.bigHash().setSizePctAndMaxItemSize(0, 100);
163+
LruAllocator::NvmCacheConfig nvmConfig;
164+
nvmConfig.navyConfig = config_;
165+
this->allocConfig_.enableNvmCache(nvmConfig);
166+
this->makeCache();
167+
161168
auto& nvm = this->cache();
162169
auto pid = this->poolId();
163170

@@ -169,6 +176,17 @@ TEST_F(NvmCacheTest, EvictToNvmGet) {
169176
auto it = nvm.allocate(pid, key, 15 * 1024);
170177
ASSERT_NE(nullptr, it);
171178
nvm.insertOrReplace(it);
179+
180+
// Ensure nvm-cache is flushed every 100 items. The reason is to
181+
// make sure we don't have any race between a "remove" operation
182+
// queued by an item's initial insertion with this item's eventual
183+
// eviction. We only flush every 100 items to avoid pushing too
184+
// many regions to flash that only has one item per region. If we
185+
// flushed per insertion, we would fill up BlockCache prematurely
186+
// and trigger evictions which are not desirable in this test.
187+
if (i % 100 == 0) {
188+
nvm.flushNvmCache();
189+
}
172190
}
173191
nvm.flushNvmCache();
174192

@@ -184,7 +202,7 @@ TEST_F(NvmCacheTest, EvictToNvmGet) {
184202
auto hdl = this->fetch(key, false /* ramOnly */);
185203
hdl.wait();
186204
if (index < nKeys) {
187-
ASSERT_NE(nullptr, hdl);
205+
ASSERT_NE(nullptr, hdl) << fmt::format("key: {}", key);
188206
// First nEvictions keys should have nvm clean bit set since
189207
// we load it from nvm
190208
const auto isClean = hdl->isNvmClean();

0 commit comments

Comments
 (0)