From 1e09a2c925188e2af1b17253a323d89c38d686c9 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 4 Oct 2021 19:39:00 +0300 Subject: [PATCH 1/3] test: Split run_mutation_source_tests There are 4 flavours of mutation source tests that are all ran sequentially -- plain, reversed and upgrade/downgrade ones that check v1<->v2 conversions. This patch splits them all into individual calls so that some tests may want to have dedicated cases for each. "By default" they are all run as they were. Signed-off-by: Pavel Emelyanov --- test/lib/mutation_source_test.cc | 17 ++++++++++++++++- test/lib/mutation_source_test.hh | 4 ++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/test/lib/mutation_source_test.cc b/test/lib/mutation_source_test.cc index 9d44b0f292..e6093ea649 100644 --- a/test/lib/mutation_source_test.cc +++ b/test/lib/mutation_source_test.cc @@ -1742,8 +1742,19 @@ void run_mutation_source_tests(populate_fn populate, bool with_partition_range_f } void run_mutation_source_tests(populate_fn_ex populate, bool with_partition_range_forwarding) { - run_mutation_reader_tests(populate, with_partition_range_forwarding); + run_mutation_source_tests_plain(populate, with_partition_range_forwarding); + run_mutation_source_tests_downgrade(populate, with_partition_range_forwarding); + run_mutation_source_tests_upgrade(populate, with_partition_range_forwarding); + run_mutation_source_tests_reverse(populate, with_partition_range_forwarding); + // Some tests call the sub-types individually, mind checking them + // if adding new stuff here +} +void run_mutation_source_tests_plain(populate_fn_ex populate, bool with_partition_range_forwarding) { + run_mutation_reader_tests(populate, with_partition_range_forwarding); +} + +void run_mutation_source_tests_downgrade(populate_fn_ex populate, bool with_partition_range_forwarding) { // ? -> v2 -> v1 -> * run_mutation_reader_tests([populate] (schema_ptr s, const std::vector& m, gc_clock::time_point t) -> mutation_source { return mutation_source([ms = populate(s, m, t)] (schema_ptr s, @@ -1758,7 +1769,9 @@ void run_mutation_source_tests(populate_fn_ex populate, bool with_partition_rang ms.make_reader_v2(s, std::move(permit), pr, slice, pc, std::move(tr), fwd, mr_fwd)); }); }, with_partition_range_forwarding); +} +void run_mutation_source_tests_upgrade(populate_fn_ex populate, bool with_partition_range_forwarding) { // ? -> v1 -> v2 -> * run_mutation_reader_tests([populate] (schema_ptr s, const std::vector& m, gc_clock::time_point t) -> mutation_source { return mutation_source([ms = populate(s, m, t)] (schema_ptr s, @@ -1773,7 +1786,9 @@ void run_mutation_source_tests(populate_fn_ex populate, bool with_partition_rang ms.make_reader(s, std::move(permit), pr, slice, pc, std::move(tr), fwd, mr_fwd)); }); }, with_partition_range_forwarding); +} +void run_mutation_source_tests_reverse(populate_fn_ex populate, bool with_partition_range_forwarding) { // read in reverse run_mutation_reader_tests([populate] (schema_ptr s, const std::vector& m, gc_clock::time_point t) -> mutation_source { auto table_schema = s->make_reversed(); diff --git a/test/lib/mutation_source_test.hh b/test/lib/mutation_source_test.hh index 650a6633be..da527fc22a 100644 --- a/test/lib/mutation_source_test.hh +++ b/test/lib/mutation_source_test.hh @@ -30,6 +30,10 @@ using populate_fn_ex = std::function Date: Mon, 4 Oct 2021 19:41:59 +0300 Subject: [PATCH 2/3] test: Split database test case The test_database_with_data_in_sstables_is_a_mutation_source case runs the mutation source tests in one go. The problem is that on each step a whole new ks:cf is created which takes the majority of the tests time. In the end of the day this case is the slowest one in the suite being up to two times longer (depending on mode) than the #2 on this list. This patch splits the case into 4 so that each mutation source flavor is run in separate case. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 113297fbde..15a78fb3a4 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -146,9 +146,9 @@ SEASTAR_TEST_CASE(test_querying_with_limits) { }); } -SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_source) { - do_with_cql_env_thread([] (cql_test_env& e) { - run_mutation_source_tests([&] (schema_ptr s, const std::vector& partitions) -> mutation_source { +static void test_database(void (*run_tests)(populate_fn_ex, bool)) { + do_with_cql_env_thread([run_tests] (cql_test_env& e) { + run_tests([&] (schema_ptr s, const std::vector& partitions, gc_clock::time_point) -> mutation_source { try { e.local_db().find_column_family(s->ks_name(), s->cf_name()); e.migration_manager().local().announce_column_family_drop(s->ks_name(), s->cf_name()).get(); @@ -172,10 +172,26 @@ SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_sourc mutation_reader::forwarding fwd_mr) { return cf.make_reader(s, std::move(permit), range, slice, pc, std::move(trace_state), fwd, fwd_mr); }); - }); + }, true); }).get(); } +SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_source_plain) { + test_database(run_mutation_source_tests_plain); +} + +SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_source_downgrade) { + test_database(run_mutation_source_tests_downgrade); +} + +SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_source_upgrade) { + test_database(run_mutation_source_tests_upgrade); +} + +SEASTAR_THREAD_TEST_CASE(test_database_with_data_in_sstables_is_a_mutation_source_reverse) { + test_database(run_mutation_source_tests_reverse); +} + SEASTAR_THREAD_TEST_CASE(test_distributed_loader_with_incomplete_sstables) { using sst = sstables::sstable; From b742e6cbb60575aebbe6de288adc058a2b27cc94 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 5 Oct 2021 11:05:40 +0300 Subject: [PATCH 3/3] test: Split multishard combining reader case All the cases in this test also run mutation source tests and the case with single-fragment buffer takes times more time to execute than the others. Splitting this single case so that it runs mutation source tests flavours in different cases improves the test parallelizm. Signed-off-by: Pavel Emelyanov --- ...ombining_reader_as_mutation_source_test.cc | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/test/boost/multishard_combining_reader_as_mutation_source_test.cc b/test/boost/multishard_combining_reader_as_mutation_source_test.cc index b16cb6c9d7..39ecc389a1 100644 --- a/test/boost/multishard_combining_reader_as_mutation_source_test.cc +++ b/test/boost/multishard_combining_reader_as_mutation_source_test.cc @@ -45,7 +45,7 @@ static std::list keep_alive_sharder; static auto make_populate(bool evict_paused_readers, bool single_fragment_buffer) { - return [evict_paused_readers, single_fragment_buffer] (schema_ptr s, const std::vector& mutations) mutable { + return [evict_paused_readers, single_fragment_buffer] (schema_ptr s, const std::vector& mutations, gc_clock::time_point) mutable { // We need to group mutations that have the same token so they land on the same shard. std::map> mutations_by_token; @@ -135,6 +135,9 @@ SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_evict_paused) { }).get(); } +// Single fragment buffer tests are extremely slow, so the +// run_mutation_source_tests execution is split + SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_with_tiny_buffer) { if (smp::count < 2) { std::cerr << "Cannot run test " << get_name() << " with smp::count < 2" << std::endl; @@ -142,7 +145,43 @@ SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_with_tiny_buffer) { } do_with_cql_env_thread([&] (cql_test_env& env) -> future<> { - run_mutation_source_tests(make_populate(true, true)); + run_mutation_source_tests_plain(make_populate(true, true), true); + return make_ready_future<>(); + }).get(); +} + +SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_with_tiny_buffer_dowgrade) { + if (smp::count < 2) { + std::cerr << "Cannot run test " << get_name() << " with smp::count < 2" << std::endl; + return; + } + + do_with_cql_env_thread([&] (cql_test_env& env) -> future<> { + run_mutation_source_tests_downgrade(make_populate(true, true), true); + return make_ready_future<>(); + }).get(); +} + +SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_with_tiny_buffer_upgrade) { + if (smp::count < 2) { + std::cerr << "Cannot run test " << get_name() << " with smp::count < 2" << std::endl; + return; + } + + do_with_cql_env_thread([&] (cql_test_env& env) -> future<> { + run_mutation_source_tests_upgrade(make_populate(true, true), true); + return make_ready_future<>(); + }).get(); +} + +SEASTAR_THREAD_TEST_CASE(test_multishard_combining_reader_with_tiny_buffer_reverse) { + if (smp::count < 2) { + std::cerr << "Cannot run test " << get_name() << " with smp::count < 2" << std::endl; + return; + } + + do_with_cql_env_thread([&] (cql_test_env& env) -> future<> { + run_mutation_source_tests_reverse(make_populate(true, true), true); return make_ready_future<>(); }).get(); }