Fix fragile CacheTest::ApplyToAllEntriesDuringResize (#10145)

Summary:
As seen in https://github.com/facebook/rocksdb/issues/10137, simply churning the cache key hashes (e.g.
by changing the raw cache keys) could trigger failure in this test, due
to possibility of some cache shard exceeding its portion of capacity
and evicting entries. Updated the test to be less fragile by using
greater margins, and added a pre-check for evictions, which doesn't
manifest as a race condition, before the main check that can race.

Also added stack trace handler to cache_test for debugging.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10145

Test Plan:
test thousands of iterations with gtest-parallel, including
with changes in https://github.com/facebook/rocksdb/issues/10137 that were surfacing the problem. Pre-check
without the fix would always fail with https://github.com/facebook/rocksdb/issues/10137

Reviewed By: guidotag

Differential Revision: D37058771

Pulled By: pdillinger

fbshipit-source-id: a7cf137967aef49c07ae9602d8523c63e7388fab

diff --git a/cache/cache_test.cc b/cache/cache_test.cc
index 735f777448c8aa04361fb35922bf0203c28cabfc..05bdc00569669dca7db3bba3d96cd8e5ee8f06a6 100644 (file)
--- a/cache/cache_test.cc
+++ b/cache/cache_test.cc
@@ -18,6 +18,7 @@
 #include "cache/clock_cache.h"
 #include "cache/fast_lru_cache.h"
 #include "cache/lru_cache.h"
+#include "port/stack_trace.h"
 #include "test_util/testharness.h"
 #include "util/coding.h"
 #include "util/string_util.h"
@@ -788,8 +789,10 @@ TEST_P(CacheTest, ApplyToAllEntriesDuringResize) {
   constexpr int kSpecialCharge = 2;
   constexpr int kNotSpecialCharge = 1;
   constexpr int kSpecialCount = 100;
+  size_t expected_usage = 0;
   for (int i = 0; i < kSpecialCount; ++i) {
     Insert(i, i * 2, kSpecialCharge);
+    expected_usage += kSpecialCharge;
   }
 
   // For callback
@@ -810,12 +813,17 @@ TEST_P(CacheTest, ApplyToAllEntriesDuringResize) {
   });
 
   // In parallel, add more entries, enough to cause resize but not enough
-  // to cause ejections
-  for (int i = kSpecialCount * 1; i < kSpecialCount * 6; ++i) {
+  // to cause ejections. (Note: if any cache shard is over capacity, there
+  // will be ejections)
+  for (int i = kSpecialCount * 1; i < kSpecialCount * 5; ++i) {
     Insert(i, i * 2, kNotSpecialCharge);
+    expected_usage += kNotSpecialCharge;
   }
 
   apply_thread.join();
+  // verify no evictions
+  ASSERT_EQ(cache_->GetUsage(), expected_usage);
+  // verify everything seen in ApplyToAllEntries
   ASSERT_EQ(special_count, kSpecialCount);
 }
 
@@ -859,6 +867,7 @@ INSTANTIATE_TEST_CASE_P(CacheTestInstance, LRUCacheTest,
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {
+  ROCKSDB_NAMESPACE::port::InstallStackTraceHandler();
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }