From 10e98b5c2eef8a7c72d2fbde28ae7268642f9566 Mon Sep 17 00:00:00 2001 From: Goran Zuzic Date: Wed, 23 Aug 2017 13:48:25 +0200 Subject: [PATCH] Fix a mistake with singleton inheritence Reviewers: sasa.stanko Reviewed By: sasa.stanko Differential Revision: https://phabricator.memgraph.io/D697 --- .../distributed/src/reactors_distributed.hpp | 5 ++-- .../distributed/src/reactors_local.hpp | 3 +- .../distributed/tests/distributed_test.cpp | 28 ++++++++++--------- .../distributed/tests/start_distributed.py | 2 +- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/experimental/distributed/src/reactors_distributed.hpp b/experimental/distributed/src/reactors_distributed.hpp index 53d866e77..0d5bd230f 100644 --- a/experimental/distributed/src/reactors_distributed.hpp +++ b/experimental/distributed/src/reactors_distributed.hpp @@ -107,7 +107,7 @@ class Network { bool success = Protocol::SendMessage(nm.address, nm.port, nm.reactor, nm.channel, std::move(nm.message)); - std::cout << "Network client message send status: " << success << std::endl; + DLOG(INFO) << "Network client message send status: " << success << std::endl; } else { this->mutex_.unlock(); } @@ -277,8 +277,9 @@ class ChannelResolvedMessage : public Message { * E.g. resolve remote channels by memgraph node id, etc. * Alive through the entire process lifetime. * Singleton class. Created automatically on first use. + * Final (can't extend) because it's a singleton. Please be careful if you're changing this. */ -class Distributed { +class Distributed final { public: /** * Get the (singleton) instance of Distributed. diff --git a/experimental/distributed/src/reactors_local.hpp b/experimental/distributed/src/reactors_local.hpp index 6cbafcb02..5e782a3a1 100644 --- a/experimental/distributed/src/reactors_local.hpp +++ b/experimental/distributed/src/reactors_local.hpp @@ -466,8 +466,9 @@ class Reactor { * E.g. holds set of reactors, channels for all reactors. * Alive through the entire process lifetime. * Singleton class. Created automatically on first use. + * Final (can't extend) because it's a singleton. Please be careful if you're changing this. */ -class System { +class System final { public: friend class Reactor; diff --git a/experimental/distributed/tests/distributed_test.cpp b/experimental/distributed/tests/distributed_test.cpp index 718b8099c..aff6928ea 100644 --- a/experimental/distributed/tests/distributed_test.cpp +++ b/experimental/distributed/tests/distributed_test.cpp @@ -8,7 +8,7 @@ DEFINE_int64(my_mnid, 0, "Memgraph node id"); // TODO(zuza): this should be assigned by the leader once in the future DEFINE_string(config_filename, "", "File containing list of all processes"); -class MemgraphDistributed : public Distributed { +class MemgraphDistributed { private: using Location = std::pair; @@ -19,8 +19,8 @@ class MemgraphDistributed : public Distributed { * More info: https://stackoverflow.com/questions/1008019/c-singleton-design-pattern */ static MemgraphDistributed &GetInstance() { - static MemgraphDistributed distributed; // guaranteed to be destroyed, initialized on first use - return distributed; + static MemgraphDistributed memgraph; // guaranteed to be destroyed, initialized on first use + return memgraph; } /** Register memgraph node id to the given location. */ @@ -34,7 +34,7 @@ class MemgraphDistributed : public Distributed { const std::string &channel) { std::unique_lock lock(mutex_); const auto &location = mnodes_.at(mnid); - return Distributed::FindChannel(location.first, location.second, reactor, channel); + return Distributed::GetInstance().FindChannel(location.first, location.second, reactor, channel); } protected: @@ -74,13 +74,13 @@ std::pair> std::string address; uint16_t port; file >> master_mnid >> address >> port; - MemgraphDistributed &distributed = MemgraphDistributed::GetInstance(); - distributed.RegisterMemgraphNode(master_mnid, address, port); + MemgraphDistributed &memgraph = MemgraphDistributed::GetInstance(); + memgraph.RegisterMemgraphNode(master_mnid, address, port); while (file.good()) { file >> mnid >> address >> port; if (file.eof()) break ; - distributed.RegisterMemgraphNode(mnid, address, port); + memgraph.RegisterMemgraphNode(mnid, address, port); worker_mnids.push_back(mnid); } file.close(); @@ -116,7 +116,9 @@ class Master : public Reactor { worker_mnids_(std::move(worker_mnids)) {} virtual void Run() { - MemgraphDistributed &distributed = MemgraphDistributed::GetInstance(); + MemgraphDistributed &memgraph = MemgraphDistributed::GetInstance(); + Distributed &distributed = Distributed::GetInstance(); + std::cout << "Master (" << mnid_ << ") @ " << distributed.network().Address() << ":" << distributed.network().Port() << std::endl; @@ -139,7 +141,7 @@ class Master : public Reactor { // send a TextMessage to each worker for (auto wmnid : worker_mnids_) { - auto stream = distributed.FindChannel(wmnid, "worker", "main"); + auto stream = memgraph.FindChannel(wmnid, "worker", "main"); stream->OnEventOnce() .ChainOnce([this, stream](const ChannelResolvedMessage &msg){ msg.channel()->Send("master", "main", "hi from master"); @@ -161,7 +163,8 @@ class Worker : public Reactor { master_mnid_(master_mnid) {} virtual void Run() { - MemgraphDistributed &distributed = MemgraphDistributed::GetInstance(); + Distributed &distributed = Distributed::GetInstance(); + std::cout << "Worker (" << mnid_ << ") @ " << distributed.network().Address() << ":" << distributed.network().Port() << std::endl; @@ -191,15 +194,14 @@ int main(int argc, char *argv[]) { gflags::ParseCommandLineFlags(&argc, &argv, true); System &system = System::GetInstance(); - MemgraphDistributed& distributed = MemgraphDistributed::GetInstance(); auto mnids = ParseConfigAndRegister(FLAGS_config_filename); - distributed.StartServices(); + Distributed::GetInstance().StartServices(); if (FLAGS_my_mnid == mnids.first) system.Spawn("master", FLAGS_my_mnid, std::move(mnids.second)); else system.Spawn("worker", FLAGS_my_mnid, mnids.first); system.AwaitShutdown(); - distributed.StopServices(); + Distributed::GetInstance().StopServices(); return 0; } diff --git a/experimental/distributed/tests/start_distributed.py b/experimental/distributed/tests/start_distributed.py index e07656272..3fb483fdb 100755 --- a/experimental/distributed/tests/start_distributed.py +++ b/experimental/distributed/tests/start_distributed.py @@ -6,7 +6,7 @@ import os command = 'gnome-terminal' program = './distributed_test' config_filename = 'config' -flags = '--minloglevel=2' +flags = '-alsologtostderr --minloglevel=2' f = open(config_filename, 'r') for line in f: