test: sstable_datafile_test: fix tracking of closed sstables in sstable_run_based_compaction_test

sstable_run_based_compaction_test assumed that sstables are freed immediately
after they are fully processed.
Hovewer, since commit b524f96a74,
mutation_reader_merger releases sstables in batches of 4, which breaks the
assumption. This fix adjusts the test accordingly.

Until now, the test only kept working by chance: by coincidence, the number of
test sstables processed by merging_reader in a single fill_buffer() call was
divisible by 4. Since the test checks happen between those calls,
the test never witnessed a situation when an sstable was fully processed,
but not released yet.

The error was noticed during the work on an upcoming patch which changes the
size of mutation_fragment, and reduces the number of test sstables processed
in a single fill_buffer() call, which breaks the test.
This commit is contained in:
Michał Chojnowski
2021-02-16 17:00:01 +01:00
parent 2f3b265dac
commit 52bd190bb3

View File

@@ -5678,7 +5678,7 @@ SEASTAR_TEST_CASE(sstable_run_based_compaction_test) {
auto tokens = token_generation_for_current_shard(16);
std::unordered_set<shared_sstable> sstables;
std::optional<utils::observer<sstable&>> observer;
std::vector<utils::observer<sstable&>> observers;
sstables::sstable_run_based_compaction_strategy_for_tests cs;
auto do_replace = [&] (const std::vector<shared_sstable>& old_sstables, const std::vector<shared_sstable>& new_sstables) {
@@ -5701,15 +5701,20 @@ SEASTAR_TEST_CASE(sstable_run_based_compaction_test) {
// check that sstable replacement follows token order
BOOST_REQUIRE(*expected_sst == old_sstables.front()->generation());
expected_sst++;
// check that previously released sstable was already closed
BOOST_REQUIRE(*closed_sstables_tracker == old_sstables.front()->generation());
// check that previously released sstables were already closed
if (old_sstables.front()->generation() % 4 == 0) {
// Due to performance reasons, sstables are not released immediately, but in batches.
// At the time of writing, mutation_reader_merger releases it's sstable references
// in batches of 4. That's why we only perform this check every 4th sstable.
BOOST_REQUIRE(*closed_sstables_tracker == old_sstables.front()->generation());
}
do_replace(old_sstables, new_sstables);
observer = old_sstables.front()->add_on_closed_handler([&] (sstable& sst) {
observers.push_back(old_sstables.front()->add_on_closed_handler([&] (sstable& sst) {
testlog.info("Closing sstable of generation {}", sst.generation());
closed_sstables_tracker++;
});
}));
testlog.info("Removing sstable of generation {}, refcnt: {}", old_sstables.front()->generation(), old_sstables.front().use_count());
};
@@ -5745,7 +5750,6 @@ SEASTAR_TEST_CASE(sstable_run_based_compaction_test) {
};
auto result = compact(std::move(desc.sstables), replacer);
observer.reset();
BOOST_REQUIRE_EQUAL(expected_output, result.size());
BOOST_REQUIRE(expected_sst == sstable_run.end());
return result;