Skip to content

Commit

Permalink
Merge 'Simplify loading_cache_test and use manual_clock' from Benny H…
Browse files Browse the repository at this point in the history 
…alevy

This series exposes a Clock template parameter for loading_cache so that the test could use
the manual_clock rather than the lowres_clock, since relying on the latter is flaky.

In addition, the test load function is simplified to sleep some small random time and co_return the expected string,
rather than reading it from a real file, since the latter's timing might also be flaky, and it out-of-scope for this test.

Fixes scylladb#20322

* The test was flaky forever, so backport is required for all live versions.

Closes scylladb#22064

* github.com:scylladb/scylladb:
  tests: loading_cache_test: use manual_clock
  utils: loading_cache: make clock_type a template parameter
  test: loading_cache_test: use function-scope loader
  test: loading_cache_test: simlute loader using sleep
  test: lib: eventually: add sleep function param
  test: lib: eventually: make *EVENTUALLY_EQUAL inline functions
  • Loading branch information
@denesb @piodul
denesb authored and piodul committed Jan 27, 2025
2 parents f911280 + 32b7cab commit 9fc14f2
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 209 deletions.
8 changes: 5 additions & 3 deletions configure.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,7 @@ def find_ninja():
'test/lib/test_utils.cc',
'test/lib/tmpdir.cc',
'test/lib/sstable_run_based_compaction_strategy_for_tests.cc',
'test/lib/eventually.cc',
]

scylla_tests_dependencies = scylla_core + alternator + idls + scylla_tests_generic_dependencies + [
Expand Down Expand Up @@ -1379,6 +1380,7 @@ def find_ninja():
'test/lib/key_utils.cc',
'test/lib/random_schema.cc',
'test/lib/data_model.cc',
'test/lib/eventually.cc',
'seastar/tests/perf/linux_perf_event.cc']

deps = {
Expand Down Expand Up @@ -1573,11 +1575,11 @@ def find_ninja():
deps['test/boost/schema_loader_test'] += ['tools/schema_loader.cc', 'tools/read_mutation.cc']
deps['test/boost/rust_test'] += ['rust/inc/src/lib.rs']

deps['test/raft/replication_test'] = ['test/raft/replication_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/raft_server_test'] = ['test/raft/raft_server_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/replication_test'] = ['test/raft/replication_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc', 'test/lib/eventually.cc'] + scylla_raft_dependencies
deps['test/raft/raft_server_test'] = ['test/raft/raft_server_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc', 'test/lib/eventually.cc'] + scylla_raft_dependencies
deps['test/raft/randomized_nemesis_test'] = ['test/raft/randomized_nemesis_test.cc', 'direct_failure_detector/failure_detector.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/failure_detector_test'] = ['test/raft/failure_detector_test.cc', 'direct_failure_detector/failure_detector.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/many_test'] = ['test/raft/many_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc'] + scylla_raft_dependencies
deps['test/raft/many_test'] = ['test/raft/many_test.cc', 'test/raft/replication.cc', 'test/raft/helpers.cc', 'test/lib/eventually.cc'] + scylla_raft_dependencies
deps['test/raft/fsm_test'] = ['test/raft/fsm_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + scylla_raft_dependencies
deps['test/raft/etcd_test'] = ['test/raft/etcd_test.cc', 'test/raft/helpers.cc', 'test/lib/log.cc'] + scylla_raft_dependencies
deps['test/raft/raft_sys_table_storage_test'] = ['test/raft/raft_sys_table_storage_test.cc'] + \
Expand Down
14 changes: 7 additions & 7 deletions test/boost/bloom_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ SEASTAR_TEST_CASE(test_sstable_manager_auto_reclaim_and_reload_of_bloom_filter)
// Test auto reload - disposing sst3 should trigger reload of the
// smallest filter in the reclaimed list, which is sst1's bloom filter.
dispose_and_stop_tracking_bf_memory(std::move(sst3), sst_mgr);
REQUIRE_EVENTUALLY_EQUAL(sst1->filter_memory_size(), sst1_bf_memory);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst1->filter_memory_size(); }, sst1_bf_memory);
// only sst4's bloom filter memory should be reported as reclaimed
REQUIRE_EVENTUALLY_EQUAL(sst_mgr.get_total_memory_reclaimed(), sst4_bf_memory);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst_mgr.get_total_memory_reclaimed(); }, sst4_bf_memory);
// sst2 and sst4 remain the same
BOOST_REQUIRE_EQUAL(sst2->filter_memory_size(), sst2_bf_memory);
BOOST_REQUIRE_EQUAL(sst4->filter_memory_size(), 0);
Expand Down Expand Up @@ -162,7 +162,7 @@ SEASTAR_TEST_CASE(test_bloom_filter_reclaim_during_reload) {
// dispose sst2 to trigger reload of sst1's bloom filter
dispose_and_stop_tracking_bf_memory(std::move(sst2), sst_mgr);
// _total_reclaimable_memory will be updated when the reload begins; wait for it.
REQUIRE_EVENTUALLY_EQUAL(sst_mgr.get_total_reclaimable_memory(), sst1_bf_memory);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst_mgr.get_total_reclaimable_memory(); }, sst1_bf_memory);

// now that the reload is midway and paused, create new sst to verify that its
// filter gets evicted immediately as the memory that became available is reserved
Expand All @@ -175,8 +175,8 @@ SEASTAR_TEST_CASE(test_bloom_filter_reclaim_during_reload) {

// resume reloading sst1 filter
utils::get_local_injector().receive_message("reload_reclaimed_components/pause");
REQUIRE_EVENTUALLY_EQUAL(sst1->filter_memory_size(), sst1_bf_memory);
REQUIRE_EVENTUALLY_EQUAL(sst_mgr.get_total_memory_reclaimed(), sst3_bf_memory);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst1->filter_memory_size(); }, sst1_bf_memory);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst_mgr.get_total_memory_reclaimed(); }, sst3_bf_memory);
BOOST_REQUIRE_EQUAL(sst_mgr.get_total_reclaimable_memory(), sst1_bf_memory);

utils::get_local_injector().disable("reload_reclaimed_components/pause");
Expand Down Expand Up @@ -275,7 +275,7 @@ SEASTAR_TEST_CASE(test_bloom_filter_reload_after_unlink) {
utils::get_local_injector().receive_message("test_bloom_filter_reload_after_unlink");
async_sst_holder.get();

REQUIRE_EVENTUALLY_EQUAL(sst_mgr.get_active_list().size(), 0);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return sst_mgr.get_active_list().size(); }, 0);
}, {
// set available memory = 0 to force reclaim the bloom filter
.available_memory = 0
Expand Down Expand Up @@ -340,7 +340,7 @@ SEASTAR_TEST_CASE(test_bloom_filter_reclaim_after_unlink) {
utils::get_local_injector().receive_message("test_bloom_filter_reload_after_unlink");
async_sst_holder.get();

REQUIRE_EVENTUALLY_EQUAL(active_list.size(), 0);
REQUIRE_EVENTUALLY_EQUAL<size_t>([&] { return active_list.size(); }, 0);
}, {
// set available memory = 0 to force reclaim the bloom filter
.available_memory = 100
Expand Down
Loading

0 comments on commit 9fc14f2

Please sign in to comment.