From 5d8543221b332d020dcad4c8e2ed105addb941db Mon Sep 17 00:00:00 2001 From: Lakshmi Narayanan Sreethar Date: Wed, 7 Aug 2024 14:51:32 +0530 Subject: [PATCH 1/2] replica: fix copy constructor of tablet_sstable_set Remove the existing copy constructor to enable the use of the implicit copy constructor. This fixes the issue of `_sstable_set_ids` not being copied in the current copy constructor. Fixes #19519 Signed-off-by: Lakshmi Narayanan Sreethar (cherry picked from commit 44583eed9e4c00f1ad60d93ed154f180f1669fd0) --- replica/tablets.cc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/replica/tablets.cc b/replica/tablets.cc index 53ae052986..b93779157d 100644 --- a/replica/tablets.cc +++ b/replica/tablets.cc @@ -378,14 +378,6 @@ public: }); } - tablet_sstable_set(const tablet_sstable_set& o) - : _schema(o._schema) - , _tablet_map(o._tablet_map.tablet_count()) - , _sstable_sets(o._sstable_sets) - , _size(o._size) - , _bytes_on_disk(o._bytes_on_disk) - {} - static lw_shared_ptr make(schema_ptr s, const storage_group_manager& sgm, const locator::tablet_map& tmap) { return make_lw_shared(std::make_unique(std::move(s), sgm, tmap)); } From ab6b8be69ab0d3eebda07c04059a46c244cf47d9 Mon Sep 17 00:00:00 2001 From: Lakshmi Narayanan Sreethar Date: Fri, 2 Aug 2024 17:22:23 +0530 Subject: [PATCH 2/2] boost/sstable_set_test: add testcase to test tablet_sstable_set copy constructor Signed-off-by: Lakshmi Narayanan Sreethar (cherry picked from commit ec47b50859f100391a575f1c36fa5e458897132a) --- test/boost/sstable_set_test.cc | 30 ++++++++++++++++++++++++++++++ test/boost/sstable_test.hh | 4 ++++ 2 files changed, 34 insertions(+) diff --git a/test/boost/sstable_set_test.cc b/test/boost/sstable_set_test.cc index ca1b1b3999..65e46e36ca 100644 --- a/test/boost/sstable_set_test.cc +++ b/test/boost/sstable_set_test.cc @@ -9,10 +9,15 @@ #include "test/lib/scylla_test_case.hh" +#include +#include "db/config.hh" +#include "locator/tablets.hh" #include "sstables/sstable_set_impl.hh" #include "sstables/shared_sstable.hh" #include "sstables/sstable_set.hh" #include "sstables/sstables.hh" +#include "sstable_test.hh" +#include "test/lib/cql_test_env.hh" #include "test/lib/simple_schema.hh" #include "test/lib/sstable_utils.hh" #include "readers/from_mutations_v2.hh" @@ -167,3 +172,28 @@ SEASTAR_TEST_CASE(test_partitioned_sstable_set_bytes_on_disk) { BOOST_REQUIRE_EQUAL(sst_set->bytes_on_disk(), ss1->bytes_on_disk() + ss2->bytes_on_disk()); }); } + +SEASTAR_TEST_CASE(test_tablet_sstable_set_copy_ctor) { + // enable tablets, to get access to tablet_storage_group_manager + cql_test_config cfg; + cfg.db_config->enable_tablets(true); + + return do_with_cql_env_thread([&](cql_test_env& env) { + env.execute_cql("CREATE KEYSPACE test_tablet_sstable_set_copy_ctor" + " WITH REPLICATION = {'class' : 'NetworkTopologyStrategy', 'replication_factor' : 1};").get(); + env.execute_cql("CREATE TABLE test_tablet_sstable_set_copy_ctor.test (pk int PRIMARY KEY);").get(); + for (int i = 0; i < 10; i++) { + env.execute_cql(fmt::format("INSERT INTO test_tablet_sstable_set_copy_ctor.test (pk) VALUES ({})", i)).get(); + } + auto& cf = env.local_db().find_column_family("test_tablet_sstable_set_copy_ctor", "test"); + auto& sgm = column_family_test::get_storage_group_manager(cf); + sgm->split_all_storage_groups().get(); + + auto tablet_sstable_set = replica::make_tablet_sstable_set(cf.schema(), *sgm.get(), locator::tablet_map(8)); + auto tablet_sstable_set_copy = *tablet_sstable_set.get(); + BOOST_REQUIRE(*tablet_sstable_set->all() == *tablet_sstable_set_copy.all()); + BOOST_REQUIRE_EQUAL(tablet_sstable_set->size(), tablet_sstable_set_copy.size()); + BOOST_REQUIRE_EQUAL(tablet_sstable_set->bytes_on_disk(), tablet_sstable_set_copy.bytes_on_disk()); + + }, std::move(cfg)); +} diff --git a/test/boost/sstable_test.hh b/test/boost/sstable_test.hh index 92e7b83c88..8b17a57349 100644 --- a/test/boost/sstable_test.hh +++ b/test/boost/sstable_test.hh @@ -53,6 +53,10 @@ public: static sstables::generation_type calculate_generation_for_new_table(replica::column_family& cf) { return cf.calculate_generation_for_new_table(); } + + static const std::unique_ptr& get_storage_group_manager(replica::column_family& cf) { + return cf._sg_manager; + } }; namespace sstables {