replica: table: Update stats for newly added SSTables

Patch 55a8421e3d fixed an inefficiency when rebuilding
statistics with many compaction groups, but it incorrectly removed
the update for newly added SSTables. This patch restores it.
When a new SSTable is added to any of the groups, the stats are
incrementally updated (as before). On compaction completion,
statistics are still rebuilt by simply iterating through each
group, which keeps track of its own stats.
Unit tests are added to guarantee the stats are correct both after
compaction completion and memtable flush.

Fixes #12808.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes #12834
This commit is contained in:
Raphael S. Carvalho
2023-02-13 14:09:52 -03:00
committed by Botond Dénes
parent cab5b08948
commit d6fe99abc4
3 changed files with 18 additions and 3 deletions

View File

@@ -543,6 +543,7 @@ private:
bool cache_enabled() const {
return _config.enable_cache && _schema->caching_options().enabled();
}
void update_stats_for_new_sstable(const sstables::shared_sstable& sst) noexcept;
future<> do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy);
// Helpers which add sstable on behalf of a compaction group and refreshes compound set.
void add_sstable(compaction_group& cg, sstables::shared_sstable sstable);

View File

@@ -546,6 +546,12 @@ future<> table::parallel_foreach_compaction_group(std::function<future<>(compact
});
}
void table::update_stats_for_new_sstable(const sstables::shared_sstable& sst) noexcept {
_stats.live_disk_space_used += sst->bytes_on_disk();
_stats.total_disk_space_used += sst->bytes_on_disk();
_stats.live_sstable_count++;
}
future<>
table::do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy) {
auto permit = co_await seastar::get_units(_sstable_set_mutation_sem, 1);
@@ -558,6 +564,7 @@ table::do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::o
} else {
add_maintenance_sstable(cg, sst);
}
update_stats_for_new_sstable(sst);
}), dht::partition_range::make({sst->get_first_decorated_key(), true}, {sst->get_last_decorated_key(), true}));
}
@@ -597,6 +604,7 @@ table::update_cache(compaction_group& cg, lw_shared_ptr<memtable> m, std::vector
auto adder = row_cache::external_updater([this, m, ssts = std::move(ssts), new_ssts_ms = std::move(*ms_opt), &cg] () mutable {
for (auto& sst : ssts) {
add_sstable(cg, sst);
update_stats_for_new_sstable(sst);
}
m->mark_flushed(std::move(new_ssts_ms));
try_trigger_compaction(cg);

View File

@@ -142,6 +142,12 @@ static std::unique_ptr<strategy_control> make_strategy_control_for_test(bool has
return std::make_unique<strategy_control_for_test>(has_ongoing_compaction);
}
static void assert_table_sstable_count(table_for_tests& t, size_t expected_count) {
testlog.info("sstable_set_size={}, live_sstable_count={}, expected={}", t->get_sstables()->size(), t->get_stats().live_sstable_count, expected_count);
BOOST_REQUIRE(t->get_sstables()->size() == expected_count);
BOOST_REQUIRE(t->get_stats().live_sstable_count == expected_count);
}
SEASTAR_TEST_CASE(compaction_manager_basic_test) {
return test_env::do_with_async([] (test_env& env) {
BOOST_REQUIRE(smp::count == 1);
@@ -3896,7 +3902,7 @@ SEASTAR_TEST_CASE(test_twcs_interposer_on_memtable_flush) {
auto expected_ssts = (split_during_flush) ? target_windows_span : 1;
testlog.info("split_during_flush={}, actual={}, expected={}", split_during_flush, cf->get_sstables()->size(), expected_ssts);
BOOST_REQUIRE(cf->get_sstables()->size() == expected_ssts);
assert_table_sstable_count(cf, expected_ssts);
};
test_interposer_on_flush(true);
@@ -4623,7 +4629,7 @@ SEASTAR_TEST_CASE(test_major_does_not_miss_data_in_memtable) {
}();
auto sst = make_sstable_containing(sst_gen, {std::move(row_mut)});
cf->add_sstable_and_update_cache(sst).get();
BOOST_REQUIRE(cf->get_sstables()->size() == 1);
assert_table_sstable_count(cf, 1);
auto deletion_mut = [&] () {
mutation m(s, pkey);
@@ -4634,7 +4640,7 @@ SEASTAR_TEST_CASE(test_major_does_not_miss_data_in_memtable) {
cf->apply(deletion_mut);
cf->compact_all_sstables().get();
BOOST_REQUIRE(cf->get_sstables()->size() == 1);
assert_table_sstable_count(cf, 1);
auto new_sst = *(cf->get_sstables()->begin());
BOOST_REQUIRE(new_sst->generation() != sst->generation());
assert_that(sstable_reader(new_sst, s, env.make_reader_permit()))