From 68d6c44e06c47271baf543d9edbc14a15b86777a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:32:57 +0100 Subject: [PATCH 1/3] Track writer request id --- nano/store/write_queue.cpp | 24 ++++++++++++++---------- nano/store/write_queue.hpp | 4 +++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/nano/store/write_queue.cpp b/nano/store/write_queue.cpp index 41b687ce76..9791a06452 100644 --- a/nano/store/write_queue.cpp +++ b/nano/store/write_queue.cpp @@ -66,7 +66,9 @@ nano::store::write_guard nano::store::write_queue::wait (writer writer) bool nano::store::write_queue::contains (writer writer) const { nano::lock_guard guard{ mutex }; - return std::find (queue.cbegin (), queue.cend (), writer) != queue.cend (); + return std::any_of (queue.cbegin (), queue.cend (), [writer] (auto const & item) { + return item.first == writer; + }); } void nano::store::write_queue::pop () @@ -83,17 +85,19 @@ void nano::store::write_queue::acquire (writer writer) { nano::unique_lock lock{ mutex }; - // There should be no duplicates in the queue - debug_assert (std::none_of (queue.cbegin (), queue.cend (), [writer] (auto const & item) { return item == writer; })); + // There should be no duplicates in the queue (exception is testing) + debug_assert (std::none_of (queue.cbegin (), queue.cend (), [writer] (auto const & item) { + return item.first == writer; + }) + || writer == writer::testing); + + auto const id = next++; // Add writer to the end of the queue if it's not already waiting - auto exists = std::find (queue.cbegin (), queue.cend (), writer) != queue.cend (); - if (!exists) - { - queue.push_back (writer); - } + queue.push_back ({ writer, id }); - condition.wait (lock, [&] () { return queue.front () == writer; }); + // Wait until we are at the front of the queue + condition.wait (lock, [&] () { return queue.front ().second == id; }); } void nano::store::write_queue::release (writer writer) @@ -101,7 +105,7 @@ void nano::store::write_queue::release (writer writer) { nano::lock_guard guard{ mutex }; release_assert (!queue.empty ()); - release_assert (queue.front () == writer); + release_assert (queue.front ().first == writer); queue.pop_front (); } condition.notify_all (); diff --git a/nano/store/write_queue.hpp b/nano/store/write_queue.hpp index 6b4688618a..0171685b87 100644 --- a/nano/store/write_queue.hpp +++ b/nano/store/write_queue.hpp @@ -70,7 +70,9 @@ class write_queue final void release (writer writer); private: - std::deque queue; + uint64_t next{ 0 }; + using entry = std::pair; // uint64_t is a unique id for each write_guard + std::deque queue; mutable nano::mutex mutex; nano::condition_variable condition; From a6d7d1ce6669b9f966602e528c884bf3d29b89d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:53:36 +0100 Subject: [PATCH 2/3] Test multithreaded transactions --- nano/core_test/ledger.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 0cb99f9a76..cbeab2c29e 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -5861,3 +5861,38 @@ TEST (ledger_transaction, write_wait_order) // Signal to continue and drop the third transaction latch3.count_down (); } + +TEST (ledger_transaction, multithreaded_interleaving) +{ + nano::test::system system; + + auto ctx = nano::test::ledger_empty (); + + int constexpr num_threads = 2; + int constexpr num_iterations = 10; + int constexpr num_blocks = 10; + + std::deque threads; + for (int i = 0; i < num_threads; ++i) + { + threads.emplace_back ([&] { + for (int n = 0; n < num_iterations; ++n) + { + auto tx = ctx.ledger ().tx_begin_write (nano::store::writer::testing); + for (unsigned k = 0; k < num_blocks; ++k) + { + ctx.store ().account.put (tx, nano::account{ k }, nano::account_info{}); + } + for (unsigned k = 0; k < num_blocks; ++k) + { + ctx.store ().account.del (tx, nano::account{ k }); + } + } + }); + } + + for (auto & thread : threads) + { + thread.join (); + } +} \ No newline at end of file From 591e0cae68e8f687b312c90d17d0c2d1b44c894c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:44:20 +0100 Subject: [PATCH 3/3] Release guard after transaction --- nano/secure/transaction.hpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/nano/secure/transaction.hpp b/nano/secure/transaction.hpp index 6c4acde3d3..8a3ebd9c1d 100644 --- a/nano/secure/transaction.hpp +++ b/nano/secure/transaction.hpp @@ -29,17 +29,18 @@ class transaction virtual operator const nano::store::transaction & () const = 0; }; -class write_transaction : public transaction +class write_transaction final : public transaction { + nano::store::write_guard guard; // Guard should be released after the transaction nano::store::write_transaction txn; - nano::store::write_guard guard; std::chrono::steady_clock::time_point start; public: - explicit write_transaction (nano::store::write_transaction && txn, nano::store::write_guard && guard) noexcept : - txn{ std::move (txn) }, - guard{ std::move (guard) } + explicit write_transaction (nano::store::write_transaction && txn_a, nano::store::write_guard && guard_a) noexcept : + guard{ std::move (guard_a) }, + txn{ std::move (txn_a) } { + debug_assert (guard.is_owned ()); start = std::chrono::steady_clock::now (); } @@ -97,7 +98,7 @@ class write_transaction : public transaction } }; -class read_transaction : public transaction +class read_transaction final : public transaction { nano::store::read_transaction txn; @@ -140,4 +141,4 @@ class read_transaction : public transaction return txn; } }; -} // namespace nano::secure +}