From 21ff6efd74517dd4642d86468bec91c7c8c00b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Mon, 17 Jul 2023 07:51:04 -0400 Subject: [PATCH] test/boost/view_build_test: improve test_view_update_generator_register_semaphore_unit_leak By making it independent of the number of units the view update generator's registration semaphore is created with. We want to increase this number significantly and that would destabilize this test significantly. To prevent this, detach the test from the number of units completely, while stil preserving the original intent behind it, as best as it could be determined. Closes #14727 --- db/view/view_update_generator.hh | 1 + test/boost/view_build_test.cc | 34 ++++++++++++++++---------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/db/view/view_update_generator.hh b/db/view/view_update_generator.hh index dbc8fd9b7f..1dcb511d54 100644 --- a/db/view/view_update_generator.hh +++ b/db/view/view_update_generator.hh @@ -85,6 +85,7 @@ public: wait_for_all_updates wait_for_all); ssize_t available_register_units() const { return _registration_sem.available_units(); } + size_t queued_batches_count() const { return _sstables_with_tables.size(); } private: bool should_throttle() const; void setup_metrics(); diff --git a/test/boost/view_build_test.cc b/test/boost/view_build_test.cc index 631415f586..fc0ed86b86 100644 --- a/test/boost/view_build_test.cc +++ b/test/boost/view_build_test.cc @@ -632,35 +632,38 @@ SEASTAR_THREAD_TEST_CASE(test_view_update_generator_register_semaphore_unit_leak return sst; }; + BOOST_REQUIRE_EQUAL(view_update_generator.queued_batches_count(), 0); + std::vector prepared_sstables; - // We need 2 * N + 1 sstables. N should be at least 5 (number of units - // on the register semaphore) + 1 (just to make sure the returned future - // blocks). While the initial batch is processed we want to register N - // more sstables, + 1 to detect the leak (N units will be returned from - // the initial batch). See below for more details. - const auto num_sstables = (view_update_generator.available_register_units() + 1) * 2 + 1; + // We need 2 * N sstables. While the initial batch is processed we want + // to register N more sstables, + 1 to detect the leak (N units will be + // returned from the initial batch). See below for more details. + const auto num_sstables = 5 * 2; for (auto i = 0; i < num_sstables; ++i) { prepared_sstables.push_back(make_sstable()); } - // First batch: register N sstables. - while (view_update_generator.available_register_units()) { + BOOST_REQUIRE_EQUAL(view_update_generator.queued_batches_count(), 0); + + for (auto i = 0; i < num_sstables / 2; ++i) { auto fut = view_update_generator.register_staging_sstable(std::move(prepared_sstables.back()), t); prepared_sstables.pop_back(); BOOST_REQUIRE(fut.available()); } - // Make sure we consumed all units and thus the register future blocks. - auto fut1 = view_update_generator.register_staging_sstable(std::move(prepared_sstables.back()), t); - prepared_sstables.pop_back(); - BOOST_REQUIRE(!fut1.available()); + BOOST_REQUIRE_EQUAL(view_update_generator.queued_batches_count(), 1); + + thread::yield(); + + // After the yield above, the first batch should have started processing. + eventually_true([&] { return view_update_generator.queued_batches_count() == 0; }); std::vector> futures; - futures.reserve(prepared_sstables.size()); + futures.reserve(num_sstables); // While the first batch is processed, concurrently register the - // remaining N + 1 sstables, yielding in-between so the first batch + // remaining N sstables, yielding in-between so the first batch // processing can progress. while (!prepared_sstables.empty()) { thread::yield(); @@ -668,9 +671,6 @@ SEASTAR_THREAD_TEST_CASE(test_view_update_generator_register_semaphore_unit_leak prepared_sstables.pop_back(); } - // Make sure the first batch is processed. - fut1.get(); - auto fut_res = when_all_succeed(futures.begin(), futures.end()); auto watchdog_timer_done = make_ready_future<>();