diff --git a/CMakeLists.txt b/CMakeLists.txt index 2a707110..0ba513f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,6 +118,7 @@ if(BUILD_TESTS) unittests/rpc/ErrorTests.cpp unittests/rpc/BaseTests.cpp unittests/rpc/RPCHelpersTest.cpp + unittests/rpc/CountersTest.cpp ## RPC handlers unittests/rpc/handlers/DefaultProcessorTests.cpp unittests/rpc/handlers/TestHandlerTests.cpp diff --git a/src/rpc/Counters.cpp b/src/rpc/Counters.cpp index c2550163..11ba6c11 100644 --- a/src/rpc/Counters.cpp +++ b/src/rpc/Counters.cpp @@ -18,33 +18,17 @@ //============================================================================== #include +#include #include #include namespace RPC { -void -Counters::initializeCounter(std::string const& method) -{ - std::shared_lock lk(mutex_); - if (methodInfo_.count(method) == 0) - { - lk.unlock(); - std::scoped_lock ulk(mutex_); - - // This calls the default constructor for methodInfo of the method. - methodInfo_[method]; - } -} - void Counters::rpcErrored(std::string const& method) { - initializeCounter(method); - - std::shared_lock lk(mutex_); + std::scoped_lock lk(mutex_); MethodInfo& counters = methodInfo_[method]; - counters.started++; counters.errored++; } @@ -52,11 +36,8 @@ Counters::rpcErrored(std::string const& method) void Counters::rpcComplete(std::string const& method, std::chrono::microseconds const& rpcDuration) { - initializeCounter(method); - - std::shared_lock lk(mutex_); + std::scoped_lock lk(mutex_); MethodInfo& counters = methodInfo_[method]; - counters.started++; counters.finished++; counters.duration += rpcDuration.count(); @@ -65,18 +46,15 @@ Counters::rpcComplete(std::string const& method, std::chrono::microseconds const void Counters::rpcForwarded(std::string const& method) { - initializeCounter(method); - - std::shared_lock lk(mutex_); + std::scoped_lock lk(mutex_); MethodInfo& counters = methodInfo_[method]; - counters.forwarded++; } boost::json::object Counters::report() const { - std::shared_lock lk(mutex_); + std::scoped_lock lk(mutex_); auto obj = boost::json::object{}; obj[JS(rpc)] = boost::json::object{}; diff --git a/src/rpc/Counters.h b/src/rpc/Counters.h index e14c9e83..bc8cdea0 100644 --- a/src/rpc/Counters.h +++ b/src/rpc/Counters.h @@ -19,12 +19,12 @@ #pragma once -#include -#include -#include -#include #include -#include + +#include + +#include +#include #include #include @@ -32,22 +32,16 @@ namespace RPC { class Counters { -private: struct MethodInfo { - MethodInfo() = default; - - std::atomic_uint64_t started{0}; - std::atomic_uint64_t finished{0}; - std::atomic_uint64_t errored{0}; - std::atomic_uint64_t forwarded{0}; - std::atomic_uint64_t duration{0}; + std::uint64_t started = 0u; + std::uint64_t finished = 0u; + std::uint64_t errored = 0u; + std::uint64_t forwarded = 0u; + std::uint64_t duration = 0u; }; - void - initializeCounter(std::string const& method); - - mutable std::shared_mutex mutex_; + mutable std::mutex mutex_; std::unordered_map methodInfo_; std::reference_wrapper workQueue_; diff --git a/src/rpc/WorkQueue.cpp b/src/rpc/WorkQueue.cpp index 3de6226f..232ee67b 100644 --- a/src/rpc/WorkQueue.cpp +++ b/src/rpc/WorkQueue.cpp @@ -27,3 +27,10 @@ WorkQueue::WorkQueue(std::uint32_t numWorkers, uint32_t maxSize) while (--numWorkers) threads_.emplace_back([this] { ioc_.run(); }); } + +WorkQueue::~WorkQueue() +{ + work_.reset(); + for (auto& thread : threads_) + thread.join(); +} diff --git a/src/rpc/WorkQueue.h b/src/rpc/WorkQueue.h index 65a40b39..1b61578c 100644 --- a/src/rpc/WorkQueue.h +++ b/src/rpc/WorkQueue.h @@ -48,6 +48,7 @@ class WorkQueue public: WorkQueue(std::uint32_t numWorkers, uint32_t maxSize = 0); + ~WorkQueue(); static WorkQueue make_WorkQueue(clio::Config const& config) diff --git a/unittests/rpc/BaseTests.cpp b/unittests/rpc/BaseTests.cpp index f7865fb7..44b70a9d 100644 --- a/unittests/rpc/BaseTests.cpp +++ b/unittests/rpc/BaseTests.cpp @@ -2,9 +2,11 @@ /* This file is part of clio: https://github.com/XRPLF/clio Copyright (c) 2023, the clio developers. + Permission to use, copy, modify, and distribute this software for any purpose with or without fee is hereby granted, provided that the above copyright notice and this permission notice appear in all copies. + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR diff --git a/unittests/rpc/CountersTest.cpp b/unittests/rpc/CountersTest.cpp new file mode 100644 index 00000000..f4de451e --- /dev/null +++ b/unittests/rpc/CountersTest.cpp @@ -0,0 +1,67 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2023, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +#include +#include + +#include +#include + +using namespace clio; +using namespace RPC; + +class RPCCountersTest : public NoLoggerFixture +{ +protected: + WorkQueue queue{4u, 1024u}; // todo: mock instead + Counters counters{queue}; +}; + +TEST_F(RPCCountersTest, CheckThatCountersAddUp) +{ + for (auto i = 0u; i < 512; ++i) + { + counters.rpcErrored("error"); + counters.rpcComplete("complete", std::chrono::milliseconds{1u}); + counters.rpcForwarded("forward"); + } + + auto const report = counters.report(); + auto const& rpc = report.at(JS(rpc)).as_object(); + + EXPECT_STREQ(rpc.at("error").as_object().at(JS(started)).as_string().c_str(), "512"); + EXPECT_STREQ(rpc.at("error").as_object().at(JS(finished)).as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("error").as_object().at(JS(errored)).as_string().c_str(), "512"); + EXPECT_STREQ(rpc.at("error").as_object().at("forwarded").as_string().c_str(), "0"); + + EXPECT_STREQ(rpc.at("complete").as_object().at(JS(started)).as_string().c_str(), "512"); + EXPECT_STREQ(rpc.at("complete").as_object().at(JS(finished)).as_string().c_str(), "512"); + EXPECT_STREQ(rpc.at("complete").as_object().at(JS(errored)).as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("complete").as_object().at("forwarded").as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("complete").as_object().at(JS(duration_us)).as_string().c_str(), "512000"); // 1000 per call + + EXPECT_STREQ(rpc.at("forward").as_object().at(JS(started)).as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("forward").as_object().at(JS(finished)).as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("forward").as_object().at(JS(errored)).as_string().c_str(), "0"); + EXPECT_STREQ(rpc.at("forward").as_object().at("forwarded").as_string().c_str(), "512"); + + EXPECT_EQ(report.at("work_queue"), queue.report()); // Counters report includes queue report +}