From 1ef11a36f41ece96124cb33345c0415faabd4fff Mon Sep 17 00:00:00 2001 From: Tyler Neely Date: Thu, 21 Jul 2022 09:33:06 +0000 Subject: [PATCH] Apply feedback from cpplint --- src/io/v3/future.hpp | 8 ++++---- src/io/v3/simulator.hpp | 16 +++++++++------- src/io/v3/simulator_handle.hpp | 28 +++++++++++++++------------- src/io/v3/transport.hpp | 8 +++----- tests/simulation/raft.cpp | 25 ++++++++++++++++++------- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/io/v3/future.hpp b/src/io/v3/future.hpp index dd909cbc4..cdc5851b1 100644 --- a/src/io/v3/future.hpp +++ b/src/io/v3/future.hpp @@ -20,7 +20,7 @@ #include "utils/logging.hpp" -#include "errors.hpp" +#include "io/v3/errors.hpp" template class MgPromise; @@ -130,7 +130,7 @@ class Shared { } public: - Shared(std::function simulator_notifier) : simulator_notifier_(simulator_notifier) {} + explicit Shared(std::function simulator_notifier) : simulator_notifier_(simulator_notifier) {} Shared() = default; Shared(Shared &&) = delete; Shared &operator=(Shared &&) = delete; @@ -141,7 +141,7 @@ class Shared { template class MgFuture { - MgFuture(std::shared_ptr> shared) : shared_(shared) {} + explicit MgFuture(std::shared_ptr> shared) : shared_(shared) {} bool consumed_or_moved_ = false; std::shared_ptr> shared_; @@ -210,7 +210,7 @@ class MgPromise { friend std::pair, MgPromise> FuturePromisePairWithNotifier(std::function); public: - MgPromise(std::shared_ptr> shared) : shared_(shared) {} + explicit MgPromise(std::shared_ptr> shared) : shared_(shared) {} MgPromise(MgPromise &&old) { shared_ = std::move(old.shared_); diff --git a/src/io/v3/simulator.hpp b/src/io/v3/simulator.hpp index fd7d3046b..067826106 100644 --- a/src/io/v3/simulator.hpp +++ b/src/io/v3/simulator.hpp @@ -11,14 +11,16 @@ #pragma once +#include +#include #include -#include "address.hpp" -#include "errors.hpp" -#include "future.hpp" -#include "simulator_config.hpp" -#include "simulator_handle.hpp" -#include "transport.hpp" +#include "io/v3/address.hpp" +#include "io/v3/errors.hpp" +#include "io/v3/future.hpp" +#include "io/v3/simulator_config.hpp" +#include "io/v3/simulator_handle.hpp" +#include "io/v3/transport.hpp" class SimulatorTransport { std::shared_ptr simulator_handle_; @@ -66,7 +68,7 @@ class Simulator { std::shared_ptr simulator_handle_; public: - Simulator(SimulatorConfig config) + explicit Simulator(SimulatorConfig config) : rng_(std::mt19937{config.rng_seed}), simulator_handle_{std::make_shared(config)} {} void ShutDown() { simulator_handle_->ShutDown(); } diff --git a/src/io/v3/simulator_handle.hpp b/src/io/v3/simulator_handle.hpp index 7a45c9980..a988991ae 100644 --- a/src/io/v3/simulator_handle.hpp +++ b/src/io/v3/simulator_handle.hpp @@ -11,23 +11,25 @@ #pragma once -#include #include + +#include #include #include #include #include #include +#include #include #include -#include "address.hpp" -#include "errors.hpp" -#include "simulator_config.hpp" -#include "simulator_stats.hpp" -#include "transport.hpp" +#include "io/v3/address.hpp" +#include "io/v3/errors.hpp" +#include "io/v3/simulator_config.hpp" +#include "io/v3/simulator_stats.hpp" +#include "io/v3/transport.hpp" -// TODO enforce this around std::any usage +// TODO(tyler) enforce this around std::any usage template concept SameAsDecayed = std::same_as>; @@ -150,9 +152,9 @@ class OpaquePromise { } template - OpaquePromise(std::unique_ptr> promise) + explicit OpaquePromise(std::unique_ptr> promise) : ti_(&typeid(T)), - ptr_((void *)promise.release()), + ptr_(static_cast(promise.release())), dtor_([](void *ptr) { static_cast *>(ptr)->~ResponsePromise(); }), is_awaited_([](void *ptr) { return static_cast *>(ptr)->IsAwaited(); }), fill_([](void *this_ptr, OpaqueMessage opaque_message) { @@ -222,7 +224,7 @@ class SimulatorHandle { SimulatorConfig config_; public: - SimulatorHandle(SimulatorConfig config) + explicit SimulatorHandle(SimulatorConfig config) : cluster_wide_time_microseconds_(config.start_time), rng_(config.rng_seed), config_(config) {} void IncrementServerCountAndWaitForQuiescentState(Address address) { @@ -254,7 +256,7 @@ class SimulatorHandle { uint64_t now = cluster_wide_time_microseconds_; for (auto &[promise_key, dop] : promises_) { - // TODO queue this up and drop it after its deadline + // TODO(tyler) queue this up and drop it after its deadline if (dop.deadline < now) { std::cout << "timing out request" << std::endl; DeadlineAndOpaquePromise dop = std::move(promises_.at(promise_key)); @@ -387,7 +389,7 @@ class SimulatorHandle { } template - requires(sizeof...(Ms) > 0) RequestResult Receive(Address &receiver, uint64_t timeout_microseconds) { + requires(sizeof...(Ms) > 0) RequestResult Receive(const Address &receiver, uint64_t timeout_microseconds) { std::unique_lock lock(mu_); uint64_t deadline = cluster_wide_time_microseconds_ + timeout_microseconds; @@ -399,7 +401,7 @@ class SimulatorHandle { OpaqueMessage message = std::move(can_rx.back()); can_rx.pop_back(); - // TODO search for item in can_receive_ that matches the desired types, rather + // TODO(tyler) search for item in can_receive_ that matches the desired types, rather // than asserting that the last item in can_rx matches. auto m_opt = message.Take(); return std::move(m_opt).value(); diff --git a/src/io/v3/transport.hpp b/src/io/v3/transport.hpp index 635b29d61..f5937ba9e 100644 --- a/src/io/v3/transport.hpp +++ b/src/io/v3/transport.hpp @@ -9,8 +9,6 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -// TODO chrono::microseconds instead of std::time_t - #pragma once #include @@ -19,9 +17,9 @@ #include "utils/result.hpp" -#include "address.hpp" -#include "errors.hpp" -#include "future.hpp" +#include "io/v3/address.hpp" +#include "io/v3/errors.hpp" +#include "io/v3/future.hpp" using memgraph::utils::BasicResult; diff --git a/tests/simulation/raft.cpp b/tests/simulation/raft.cpp index aa6e8bd5a..d476d8087 100644 --- a/tests/simulation/raft.cpp +++ b/tests/simulation/raft.cpp @@ -118,15 +118,26 @@ struct Follower { using Role = std::variant; -template +/* +template +concept ReplicatedStateMachine = true; +requires(T a, uint8_t *ptr, size_t len) { + { a.Serialize() } -> std::same_as>; + { T::Deserialize(ptr, len) } -> std::same_as; +}; +*/ + +template Rsm*/> class Server { CommonState state_; Role role_ = Candidate{}; Io io_; std::vector
peers_; + // Rsm rsm_; public: - Server(Io io, std::vector
peers) : io_(io), peers_(peers) {} + Server(Io &&io, std::vector
peers /*, Rsm &&rsm */) + : io_(std::move(io)), peers_(peers) /*, rsm_(std::move(rsm)*/ {} void Run() { Time last_cron = io_.Now(); @@ -622,7 +633,7 @@ void RunServer(Server server) { } void RunSimulation() { - auto config = SimulatorConfig{ + SimulatorConfig config{ .drop_percent = 5, .perform_timeouts = true, .scramble_messages = true, @@ -647,9 +658,9 @@ void RunSimulation() { std::vector
srv_2_peers = {srv_addr_1, srv_addr_3}; std::vector
srv_3_peers = {srv_addr_1, srv_addr_2}; - Server srv_1{srv_io_1, srv_1_peers}; - Server srv_2{srv_io_2, srv_2_peers}; - Server srv_3{srv_io_3, srv_3_peers}; + Server srv_1{std::move(srv_io_1), srv_1_peers}; + Server srv_2{std::move(srv_io_2), srv_2_peers}; + Server srv_3{std::move(srv_io_3), srv_3_peers}; auto srv_thread_1 = std::jthread(RunServer, std::move(srv_1)); simulator.IncrementServerCountAndWaitForQuiescentState(srv_addr_1); @@ -662,7 +673,7 @@ void RunSimulation() { std::cout << "beginning test after servers have become quiescent" << std::endl; - std::mt19937 cli_rng_{}; + std::mt19937 cli_rng_{0}; Address server_addrs[]{srv_addr_1, srv_addr_2, srv_addr_3}; bool success = false; Address leader = server_addrs[0];