Skip to content

Commit 28a69bf

Browse files
rlyerlymeta-codesync[bot]
authored andcommitted
Remove sleep that waits for slab advising workers
Summary: Even though this test looks [healthy on TestX](https://www.internalfb.com/intern/test/844425196455282) it's marked flaky. Here are the errors: {F1988281045} The issue seems to be that we're waiting 5 seconds for locking/advising, but sometimes the process hasn't finished by the time we get to the asserts. That jives with this issue showing up in stress runs (18 jobs) on the dev (slow) builds. Use the ASSERT_EVENTUALLY_TRUE helper instead. While it internally does sleeping, it's more robust to these sorts of transient issues. Also remove an unused header. ___ overriding_review_checks_triggers_an_audit_and_retroactive_review Oncall Short Name: cachelib Differential Revision: D100989998 fbshipit-source-id: 01d4c98fe4ea5dbcb6e3cbc6eb0e93b5293b06d7
1 parent f3dd264 commit 28a69bf

2 files changed

Lines changed: 7 additions & 8 deletions

File tree

cachelib/allocator/memory/tests/SlabAllocatorTest.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,6 @@ void testAdvise(SlabAllocator& s,
646646
const size_t numAdviseSlabs,
647647
void* memory,
648648
const size_t size) {
649-
ASSERT_EQ(util::getNumResidentPages(memory, size), util::getNumPages(size));
650649
auto memRssBefore = facebook::cachelib::util::getRSSBytes();
651650

652651
// Use up all but a few slabs so that we have a few free
@@ -672,8 +671,6 @@ void testRestoreAndAdvise(SlabAllocator& s,
672671
const size_t numAdviseSlabs,
673672
void* memory,
674673
const size_t size) {
675-
ASSERT_EQ(util::getNumResidentPages(memory, size),
676-
util::getNumPages(size - numAdviseSlabs * Slab::kSize));
677674
auto memRssBefore = facebook::cachelib::util::getRSSBytes();
678675

679676
// No free slabs available
@@ -720,17 +717,20 @@ TEST_F(SlabAllocatorTest, AdviseSaveRestore) {
720717
{
721718
SlabAllocator s(memory, size, config);
722719
// Wait until memory locking completes.
723-
/* sleep override */
724-
std::this_thread::sleep_for(std::chrono::seconds(5));
720+
ASSERT_EVENTUALLY_TRUE([=]() {
721+
return util::getNumResidentPages(memory, size) == util::getNumPages(size);
722+
});
725723
testAdvise(s, numSlabs, numAdviseSlabs, memory, size);
726724
state = s.saveState();
727725
}
728726

729727
{
730728
SlabAllocator r(state, memory, size, config);
731729
// Wait until memory locking completes.
732-
/* sleep override */
733-
std::this_thread::sleep_for(std::chrono::seconds(5));
730+
ASSERT_EVENTUALLY_TRUE([=]() {
731+
return util::getNumResidentPages(memory, size) ==
732+
util::getNumPages(size - numAdviseSlabs * Slab::kSize);
733+
});
734734
testRestoreAndAdvise(r, numAdviseSlabs, memory, size);
735735
}
736736
}

cachelib/common/TestUtils.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
#include <cstdint>
2020
#include <functional>
21-
#include <set>
2221
#include <string>
2322

2423
namespace facebook {

0 commit comments

Comments
 (0)