bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

sstable_set: Fix partitioned_sstable_set constructor

Browse Source 
The sstable set param isn't being used anywhere, and it's also buggy
as sstable run list isn't being updated accordingly. so it could happen
that set contains sstables but run list is empty, introducing
inconsistency.

we're fortunate that the bug wasn't activated as it would've been
a hard one to catch. found this while auditting the code.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220617203438.74336-1-raphaelsc@scylladb.com>
This commit is contained in:
Raphael S. Carvalho
2022-06-17 17:34:38 -03:00
committed by Avi Kivity
parent 59acc58920
commit aa667e590e
9 changed files with 20 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
compaction/compaction.cc
View File

@@ -897,7 +897,7 @@ public:
}
virtual sstables::sstable_set make_sstable_set_for_input() const override {
return sstables::make_partitioned_sstable_set(_schema, make_lw_shared<sstable_list>(sstable_list{}), false);
return sstables::make_partitioned_sstable_set(_schema, false);
}
flat_mutation_reader_v2 make_sstable_reader() const override {
@@ -1692,7 +1692,7 @@ static future<compaction_result> scrub_sstables_validate_mode(sstables::compacti
auto schema = table_s.schema();
formatted_sstables_list sstables_list_msg;
auto sstables = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(schema, make_lw_shared<sstable_list>(sstable_list{}), false));
auto sstables = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(schema, false));
for (const auto& sst : descriptor.sstables) {
sstables_list_msg += sst;
sstables->insert(sst);

2
db/view/view_update_generator.cc
View File

@@ -58,7 +58,7 @@ future<> view_update_generator::start() {
// Exploit the fact that sstables in the staging directory
// are usually non-overlapping and use a partitioned set for
// the read.
auto ssts = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(s, make_lw_shared<sstable_list>(sstable_list{}), false));
auto ssts = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(s, false));
for (auto& sst : sstables) {
ssts->insert(sst);
}

2
replica/table.cc
View File

@@ -97,7 +97,7 @@ lw_shared_ptr<sstables::sstable_set> table::make_maintenance_sstable_set() const
// Level metadata is not used because (level 0) maintenance sstables are disjoint and must be stored for efficient retrieval in the partitioned set
bool use_level_metadata = false;
return make_lw_shared<sstables::sstable_set>(
sstables::make_partitioned_sstable_set(_schema, make_lw_shared<sstable_list>(sstable_list{}), use_level_metadata));
sstables::make_partitioned_sstable_set(_schema, use_level_metadata));
}
void table::refresh_compound_sstable_set() {

12
sstables/sstable_set.cc
View File

@@ -224,9 +224,9 @@ dht::partition_range partitioned_sstable_set::to_partition_range(const dht::ring
return dht::partition_range::make(std::move(lower_bound), std::move(upper_bound));
}
partitioned_sstable_set::partitioned_sstable_set(schema_ptr schema, lw_shared_ptr<sstable_list> all, bool use_level_metadata)
partitioned_sstable_set::partitioned_sstable_set(schema_ptr schema, bool use_level_metadata)
: _schema(std::move(schema))
, _all(std::move(all))
, _all(make_lw_shared<sstable_list>())
, _use_level_metadata(use_level_metadata) {
}
@@ -589,19 +589,19 @@ std::unique_ptr<incremental_selector_impl> partitioned_sstable_set::make_increme
std::unique_ptr<sstable_set_impl> compaction_strategy_impl::make_sstable_set(schema_ptr schema) const {
// with use_level_metadata enabled, L0 sstables will not go to interval map, which suits well STCS.
return std::make_unique<partitioned_sstable_set>(schema, make_lw_shared<sstable_list>(), true);
return std::make_unique<partitioned_sstable_set>(schema, true);
}
std::unique_ptr<sstable_set_impl> leveled_compaction_strategy::make_sstable_set(schema_ptr schema) const {
return std::make_unique<partitioned_sstable_set>(std::move(schema), make_lw_shared<sstable_list>());
return std::make_unique<partitioned_sstable_set>(std::move(schema));
}
std::unique_ptr<sstable_set_impl> time_window_compaction_strategy::make_sstable_set(schema_ptr schema) const {
return std::make_unique<time_series_sstable_set>(std::move(schema));
}
sstable_set make_partitioned_sstable_set(schema_ptr schema, lw_shared_ptr<sstable_list> all, bool use_level_metadata) {
return sstable_set(std::make_unique<partitioned_sstable_set>(schema, std::move(all), use_level_metadata), schema);
sstable_set make_partitioned_sstable_set(schema_ptr schema, bool use_level_metadata) {
return sstable_set(std::make_unique<partitioned_sstable_set>(schema, use_level_metadata), schema);
}
sstable_set

2
sstables/sstable_set.hh
View File

@@ -144,7 +144,7 @@ public:
friend class compound_sstable_set;
};
sstable_set make_partitioned_sstable_set(schema_ptr schema, lw_shared_ptr<sstable_list> all, bool use_level_metadata = true);
sstable_set make_partitioned_sstable_set(schema_ptr schema, bool use_level_metadata = true);
sstable_set make_compound_sstable_set(schema_ptr schema, std::vector<lw_shared_ptr<sstable_set>> sets);

2
sstables/sstable_set_impl.hh
View File

@@ -79,7 +79,7 @@ public:
static dht::partition_range to_partition_range(const dht::ring_position_view& pos, const interval_type& i);
partitioned_sstable_set(const partitioned_sstable_set&) = delete;
explicit partitioned_sstable_set(schema_ptr schema, lw_shared_ptr<sstable_list> all, bool use_level_metadata = true);
explicit partitioned_sstable_set(schema_ptr schema, bool use_level_metadata = true);
// For cloning the partitioned_sstable_set (makes a deep copy, including *_all)
explicit partitioned_sstable_set(
schema_ptr schema,

3
sstables_loader.cc
View File

@@ -128,8 +128,7 @@ future<> sstables_loader::load_and_stream(sstring ks_name, sstring cf_name,
size_t nr_sst_current = 0;
while (!sstables.empty()) {
auto ops_uuid = utils::make_random_uuid();
auto sst_set = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(s,
make_lw_shared<sstable_list>(sstable_list{}), false));
auto sst_set = make_lw_shared<sstables::sstable_set>(sstables::make_partitioned_sstable_set(s, false));
size_t batch_sst_nr = 16;
std::vector<sstring> sst_names;
std::vector<sstables::shared_sstable> sst_processed;

4
test/boost/sstable_compaction_test.cc
View File

@@ -4654,8 +4654,8 @@ SEASTAR_TEST_CASE(compound_sstable_set_incremental_selector_test) {
};
auto incremental_selection_test = [&] (strategy_param param) {
auto set1 = make_lw_shared<sstable_set>(sstables::make_partitioned_sstable_set(s, make_lw_shared<sstable_list>(), false));
auto set2 = make_lw_shared<sstable_set>(sstables::make_partitioned_sstable_set(s, make_lw_shared<sstable_list>(), bool(param)));
auto set1 = make_lw_shared<sstable_set>(sstables::make_partitioned_sstable_set(s, false));
auto set2 = make_lw_shared<sstable_set>(sstables::make_partitioned_sstable_set(s, bool(param)));
set1->insert(sstable_for_overlapping_test(env, s, 0, key_and_token_pair[1].first, key_and_token_pair[1].first, 1));
set2->insert(sstable_for_overlapping_test(env, s, 1, key_and_token_pair[0].first, key_and_token_pair[2].first, 1));
set2->insert(sstable_for_overlapping_test(env, s, 2, key_and_token_pair[3].first, key_and_token_pair[3].first, 1));

6
test/boost/sstable_set_test.cc
View File

@@ -16,7 +16,11 @@
#include "readers/from_mutations_v2.hh"
static sstables::sstable_set make_sstable_set(schema_ptr schema, lw_shared_ptr<sstable_list> all = {}, bool use_level_metadata = true) {
return sstables::sstable_set(std::make_unique<partitioned_sstable_set>(schema, std::move(all), use_level_metadata), schema);
auto ret = sstables::sstable_set(std::make_unique<partitioned_sstable_set>(schema, use_level_metadata), schema);
for (auto& sst : *all) {
ret.insert(sst);
}
return ret;
}
SEASTAR_TEST_CASE(test_sstables_sstable_set_read_modify_write) {