diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2019-05-10 11:25:48 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2019-05-10 12:25:38 +0000 |
commit | 4412aace869986be3a1060f78f367841353d3384 (patch) | |
tree | f4b5e1f6da5eaf1563f3b2fd64779800acfd5796 /config | |
parent | 840d4e0578dc627b75bcd0050f1b253e84cc30ed (diff) |
Simplify the supervisor responsibility
Diffstat (limited to 'config')
-rw-r--r-- | config/src/apps/vespa-configproxy-cmd/proxycmd.cpp | 30 | ||||
-rw-r--r-- | config/src/apps/vespa-configproxy-cmd/proxycmd.h | 3 | ||||
-rw-r--r-- | config/src/apps/vespa-get-config/getconfig.cpp | 36 | ||||
-rw-r--r-- | config/src/apps/vespa-ping-configproxy/pingproxy.cpp | 24 | ||||
-rw-r--r-- | config/src/tests/failover/failover.cpp | 19 | ||||
-rw-r--r-- | config/src/tests/file_acquirer/file_acquirer_test.cpp | 16 | ||||
-rw-r--r-- | config/src/tests/frt/frt.cpp | 14 | ||||
-rw-r--r-- | config/src/vespa/config/file_acquirer/file_acquirer.cpp | 10 | ||||
-rw-r--r-- | config/src/vespa/config/file_acquirer/file_acquirer.h | 6 | ||||
-rw-r--r-- | config/src/vespa/config/frt/frtconnectionpool.cpp | 9 | ||||
-rw-r--r-- | config/src/vespa/config/frt/frtconnectionpool.h | 5 |
11 files changed, 98 insertions, 74 deletions
diff --git a/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp b/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp index 3407a880ec7..ce0665eae4a 100644 --- a/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp +++ b/config/src/apps/vespa-configproxy-cmd/proxycmd.cpp @@ -16,41 +16,41 @@ Flags::Flags() targethost("localhost"), portnumber(19090) { } -Flags::~Flags() { } +Flags::~Flags() = default; ProxyCmd::ProxyCmd(const Flags& flags) - : _supervisor(NULL), - _target(NULL), - _req(NULL), + : _server(), + _supervisor(nullptr), + _target(nullptr), + _req(nullptr), _flags(flags) { } -ProxyCmd::~ProxyCmd() { } +ProxyCmd::~ProxyCmd() = default; void ProxyCmd::initRPC() { - _supervisor = new FRT_Supervisor(); + _server = std::make_unique<fnet::frt::StandaloneFRT>(); + _supervisor = &_server->supervisor(); _req = _supervisor->AllocRPCRequest(); - _supervisor->Start(); } void ProxyCmd::invokeRPC() { - if (_req == NULL) return; + if (_req == nullptr) return; _target->InvokeSync(_req, 65.0); } void ProxyCmd::finiRPC() { - if (_req != NULL) { + if (_req != nullptr) { _req->SubRef(); - _req = NULL; + _req = nullptr; } - if (_target != NULL) { + if (_target != nullptr) { _target->SubRef(); _target = NULL; } - if (_supervisor != NULL) { - _supervisor->ShutDown(true); - delete _supervisor; - _supervisor = NULL; + if (_server) { + _server.reset(); + _supervisor = nullptr; } } diff --git a/config/src/apps/vespa-configproxy-cmd/proxycmd.h b/config/src/apps/vespa-configproxy-cmd/proxycmd.h index df8f429a3c9..49be0fff885 100644 --- a/config/src/apps/vespa-configproxy-cmd/proxycmd.h +++ b/config/src/apps/vespa-configproxy-cmd/proxycmd.h @@ -9,6 +9,8 @@ class FRT_Target; class FRT_RPCRequest; class FRT_Values; +namespace fnet::frt { class StandaloneFRT; } + struct Flags { vespalib::string method; std::vector<vespalib::string> args; @@ -23,6 +25,7 @@ struct Flags { class ProxyCmd { private: + std::unique_ptr<fnet::frt::StandaloneFRT> _server; FRT_Supervisor *_supervisor; FRT_Target *_target; FRT_RPCRequest *_req; diff --git a/config/src/apps/vespa-get-config/getconfig.cpp b/config/src/apps/vespa-get-config/getconfig.cpp index 290beacf5a4..f23f28b2734 100644 --- a/config/src/apps/vespa-get-config/getconfig.cpp +++ b/config/src/apps/vespa-get-config/getconfig.cpp @@ -20,6 +20,7 @@ using namespace config; class GetConfig : public FastOS_Application { private: + std::unique_ptr<fnet::frt::StandaloneFRT> _server; FRT_Supervisor *_supervisor; FRT_Target *_target; @@ -27,7 +28,7 @@ private: GetConfig &operator=(const GetConfig &); public: - GetConfig() : _supervisor(NULL), _target(NULL) {} + GetConfig() : _server(), _supervisor(nullptr), _target(nullptr) {} virtual ~GetConfig(); int usage(); void initRPC(const char *spec); @@ -38,8 +39,8 @@ public: GetConfig::~GetConfig() { - LOG_ASSERT(_supervisor == NULL); - LOG_ASSERT(_target == NULL); + LOG_ASSERT(_supervisor == nullptr); + LOG_ASSERT(_target == nullptr); } @@ -71,23 +72,22 @@ GetConfig::usage() void GetConfig::initRPC(const char *spec) { - _supervisor = new FRT_Supervisor(); + _server = std::make_unique<fnet::frt::StandaloneFRT>(); + _supervisor = &_server->supervisor(); _target = _supervisor->GetTarget(spec); - _supervisor->Start(); } void GetConfig::finiRPC() { - if (_target != NULL) { + if (_target != nullptr) { _target->SubRef(); - _target = NULL; + _target = nullptr; } - if (_supervisor != NULL) { - _supervisor->ShutDown(true); - delete _supervisor; - _supervisor = NULL; + if (_server) { + _server.reset(); + _supervisor = nullptr; } } @@ -99,8 +99,8 @@ GetConfig::Main() char c = -1; std::vector<vespalib::string> defSchema; - const char *schema = NULL; - const char *defName = NULL; + const char *schema = nullptr; + const char *defName = nullptr; const char *defMD5 = ""; std::string defNamespace("config"); const char *serverHost = "localhost"; @@ -110,7 +110,7 @@ GetConfig::Main() const char *vespaVersionString = nullptr; int64_t generation = 0; - if (configId == NULL) { + if (configId == nullptr) { configId = ""; } const char *configMD5 = ""; @@ -119,7 +119,7 @@ GetConfig::Main() int serverPort = 19090; - const char *optArg = NULL; + const char *optArg = nullptr; int optInd = 0; while ((c = GetOpt("a:n:v:g:i:jlm:c:t:V:w:r:s:p:dh", optArg, optInd)) != -1) { int retval = 1; @@ -181,19 +181,19 @@ GetConfig::Main() } } - if (defName == NULL || serverPort == 0) { + if (defName == nullptr || serverPort == 0) { usage(); return 1; } - if (strchr(defName, '.') != NULL) { + if (strchr(defName, '.') != nullptr) { const char *tmp = defName; defName = strrchr(defName, '.'); defName++; defNamespace = std::string(tmp, defName - tmp - 1); } - if (schema != NULL) { + if (schema != nullptr) { std::ifstream is; is.open(schema); std::string item; diff --git a/config/src/apps/vespa-ping-configproxy/pingproxy.cpp b/config/src/apps/vespa-ping-configproxy/pingproxy.cpp index a88d7deb79a..d681473ce34 100644 --- a/config/src/apps/vespa-ping-configproxy/pingproxy.cpp +++ b/config/src/apps/vespa-ping-configproxy/pingproxy.cpp @@ -12,6 +12,7 @@ LOG_SETUP("vespa-ping-configproxy"); class PingProxy : public FastOS_Application { private: + std::unique_ptr<fnet::frt::StandaloneFRT> _server; FRT_Supervisor *_supervisor; FRT_Target *_target; @@ -19,7 +20,7 @@ private: PingProxy &operator=(const PingProxy &); public: - PingProxy() : _supervisor(NULL), _target(NULL) {} + PingProxy() : _server(), _supervisor(nullptr), _target(nullptr) {} virtual ~PingProxy(); int usage(); void initRPC(const char *spec); @@ -30,8 +31,8 @@ public: PingProxy::~PingProxy() { - LOG_ASSERT(_supervisor == NULL); - LOG_ASSERT(_target == NULL); + LOG_ASSERT(_supervisor == nullptr); + LOG_ASSERT(_target == nullptr); } @@ -48,23 +49,22 @@ PingProxy::usage() void PingProxy::initRPC(const char *spec) { - _supervisor = new FRT_Supervisor(); + _server = std::make_unique<fnet::frt::StandaloneFRT>(); + _supervisor = &_server->supervisor(); _target = _supervisor->GetTarget(spec); - _supervisor->Start(); } void PingProxy::finiRPC() { - if (_target != NULL) { + if (_target != nullptr) { _target->SubRef(); - _target = NULL; + _target = nullptr; } - if (_supervisor != NULL) { - _supervisor->ShutDown(true); - delete _supervisor; - _supervisor = NULL; + if (_server) { + _server.reset(); + _supervisor = nullptr; } } @@ -80,7 +80,7 @@ PingProxy::Main() int clientTimeout = 5; int serverPort = 19090; - const char *optArg = NULL; + const char *optArg = nullptr; int optInd = 0; while ((c = GetOpt("w:s:p:dh", optArg, optInd)) != -1) { switch (c) { diff --git a/config/src/tests/failover/failover.cpp b/config/src/tests/failover/failover.cpp index 990ca761e7e..7e7017ff057 100644 --- a/config/src/tests/failover/failover.cpp +++ b/config/src/tests/failover/failover.cpp @@ -34,7 +34,7 @@ struct RPCServer : public FRT_Invokable { vespalib::Barrier barrier; int64_t gen; - RPCServer() : supervisor(NULL), barrier(2), gen(1) { } + RPCServer() : supervisor(nullptr), barrier(2), gen(1) { } void init(FRT_Supervisor * s) { FRT_ReflectionBuilder rb(s); @@ -81,18 +81,20 @@ struct RPCServer : public FRT_Invokable { void verifyConfig(std::unique_ptr<MyConfig> config) { - ASSERT_TRUE(config.get() != NULL); + ASSERT_TRUE(config); ASSERT_EQUAL("myval", config->myField); } struct ServerFixture { using UP = std::unique_ptr<ServerFixture>; + std::unique_ptr<fnet::frt::StandaloneFRT> frtServer; FRT_Supervisor * supervisor; RPCServer server; Barrier b; const vespalib::string listenSpec; ServerFixture(const vespalib::string & ls) - : supervisor(NULL), + : frtServer(), + supervisor(nullptr), server(), b(2), listenSpec(ls) @@ -106,22 +108,21 @@ struct ServerFixture { void start() { - supervisor = new FRT_Supervisor(); + frtServer = std::make_unique<fnet::frt::StandaloneFRT>(); + supervisor = & frtServer->supervisor(); server.init(supervisor); supervisor->Listen(get_port(listenSpec)); wait(); // Wait until test runner signals we can start - supervisor->Main(); wait(); // Signalling that we have shut down wait(); // Wait for signal saying that supervisor is deleted } void stop() { - if (supervisor != NULL) { - supervisor->ShutDown(true); + if (frtServer) { wait(); // Wait for supervisor to shut down - delete supervisor; - supervisor = NULL; + frtServer.reset(); + supervisor = nullptr; wait(); // Signal that we are done and start can return. } } diff --git a/config/src/tests/file_acquirer/file_acquirer_test.cpp b/config/src/tests/file_acquirer/file_acquirer_test.cpp index 0453c6ddbd0..33bb8f47e09 100644 --- a/config/src/tests/file_acquirer/file_acquirer_test.cpp +++ b/config/src/tests/file_acquirer/file_acquirer_test.cpp @@ -7,8 +7,10 @@ using namespace config; struct ServerFixture : FRT_Invokable { - FRT_Supervisor orb; + fnet::frt::StandaloneFRT server; + FRT_Supervisor &orb; vespalib::string spec; + void init_rpc() { FRT_ReflectionBuilder rb(&orb); rb.DefineMethod("waitFor", "s", "s", FRT_METHOD(ServerFixture::RPC_waitFor), this); @@ -16,24 +18,24 @@ struct ServerFixture : FRT_Invokable { rb.ParamDesc("file_ref", "file reference to wait for and resolve"); rb.ReturnDesc("file_path", "actual path to the requested file"); } - ServerFixture() : orb() { + + ServerFixture() : server(), orb(server.supervisor()) { init_rpc(); orb.Listen(0); spec = vespalib::make_string("tcp/localhost:%d", orb.GetListenPort()); - orb.Start(); } + void RPC_waitFor(FRT_RPCRequest *req) { FRT_Values ¶ms = *req->GetParams(); - FRT_Values &ret = *req->GetReturn(); + FRT_Values &ret = *req->GetReturn(); if (strcmp(params[0]._string._str, "my_ref") == 0) { ret.AddString("my_path"); } else { req->SetError(FRTE_RPC_METHOD_FAILED, "invalid file reference"); } } - ~ServerFixture() { - orb.ShutDown(true); - } + + ~ServerFixture() = default; }; TEST_FF("require that files can be acquired over rpc", ServerFixture(), RpcFileAcquirer(f1.spec)) { diff --git a/config/src/tests/frt/frt.cpp b/config/src/tests/frt/frt.cpp index c225dd9dcf3..f489ca4c7d9 100644 --- a/config/src/tests/frt/frt.cpp +++ b/config/src/tests/frt/frt.cpp @@ -119,17 +119,18 @@ namespace { int errorCode; int timeout; FRT_RPCRequest * ans; - FRT_Supervisor supervisor; + fnet::frt::StandaloneFRT server; + FRT_Supervisor & supervisor; FNET_Scheduler scheduler; vespalib::string address; - ConnectionMock(FRT_RPCRequest * answer = NULL); + ConnectionMock(FRT_RPCRequest * answer = nullptr); ~ConnectionMock(); FRT_RPCRequest * allocRPCRequest() override { return supervisor.AllocRPCRequest(); } void setError(int ec) override { errorCode = ec; } void invoke(FRT_RPCRequest * req, double t, FRT_IRequestWait * waiter) override { timeout = static_cast<int>(t); - if (ans != NULL) + if (ans != nullptr) waiter->RequestDone(ans); else waiter->RequestDone(req); @@ -142,10 +143,11 @@ namespace { : errorCode(0), timeout(0), ans(answer), - supervisor(), + server(), + supervisor(server.supervisor()), address() { } - ConnectionMock::~ConnectionMock() { } + ConnectionMock::~ConnectionMock() = default; struct FactoryMock : public ConnectionFactory { ConnectionMock * current; @@ -284,7 +286,7 @@ TEST("require that v3 request is correctly initialized") { ConfigDefinition origDef(MyConfig::CONFIG_DEF_SCHEMA); FRT_RPCRequest * req = v3req.getRequest(); - ASSERT_TRUE(req != NULL); + ASSERT_TRUE(req != nullptr); FRT_Values & params(*req->GetParams()); std::string json(params[0]._string._str); Slime slime; diff --git a/config/src/vespa/config/file_acquirer/file_acquirer.cpp b/config/src/vespa/config/file_acquirer/file_acquirer.cpp index 140d63aa7e6..a61f480b33f 100644 --- a/config/src/vespa/config/file_acquirer/file_acquirer.cpp +++ b/config/src/vespa/config/file_acquirer/file_acquirer.cpp @@ -4,6 +4,8 @@ #include <vespa/fnet/frt/supervisor.h> #include <vespa/fnet/frt/target.h> #include <vespa/fnet/frt/rpcrequest.h> +#include <vespa/fnet/transport.h> +#include <vespa/fastos/thread.h> #include <vespa/log/log.h> LOG_SETUP(".config.file_acquirer"); @@ -11,10 +13,12 @@ LOG_SETUP(".config.file_acquirer"); namespace config { RpcFileAcquirer::RpcFileAcquirer(const vespalib::string &spec) - : _orb(std::make_unique<FRT_Supervisor>()), + : _threadPool(std::make_unique<FastOS_ThreadPool>(1024*60)), + _transport(std::make_unique<FNET_Transport>()), + _orb(std::make_unique<FRT_Supervisor>(_transport.get())), _spec(spec) { - _orb->Start(); + _transport->Start(_threadPool.get()); } vespalib::string @@ -39,7 +43,7 @@ RpcFileAcquirer::wait_for(const vespalib::string &file_ref, double timeout_s) RpcFileAcquirer::~RpcFileAcquirer() { - _orb->ShutDown(true); + _transport->ShutDown(true); } } // namespace config diff --git a/config/src/vespa/config/file_acquirer/file_acquirer.h b/config/src/vespa/config/file_acquirer/file_acquirer.h index 77b0cf9a821..844e277cbf9 100644 --- a/config/src/vespa/config/file_acquirer/file_acquirer.h +++ b/config/src/vespa/config/file_acquirer/file_acquirer.h @@ -4,6 +4,8 @@ #include <vespa/vespalib/stllike/string.h> class FRT_Supervisor; +class FNET_Transport; +class FastOS_ThreadPool; namespace config { @@ -23,12 +25,14 @@ struct FileAcquirer { class RpcFileAcquirer : public FileAcquirer { private: + std::unique_ptr<FastOS_ThreadPool> _threadPool; + std::unique_ptr<FNET_Transport> _transport; std::unique_ptr<FRT_Supervisor> _orb; vespalib::string _spec; public: RpcFileAcquirer(const vespalib::string &spec); vespalib::string wait_for(const vespalib::string &file_ref, double timeout_s) override; - ~RpcFileAcquirer(); + ~RpcFileAcquirer() override; }; } // namespace config diff --git a/config/src/vespa/config/frt/frtconnectionpool.cpp b/config/src/vespa/config/frt/frtconnectionpool.cpp index 4bea8062a03..b7440ceb7f0 100644 --- a/config/src/vespa/config/frt/frtconnectionpool.cpp +++ b/config/src/vespa/config/frt/frtconnectionpool.cpp @@ -4,6 +4,7 @@ #include <vespa/vespalib/util/host_name.h> #include <vespa/fnet/frt/supervisor.h> #include <vespa/fnet/transport.h> +#include <vespa/fastos/thread.h> namespace config { @@ -26,7 +27,9 @@ FRTConnectionPool::FRTConnectionKey::operator==(const FRTConnectionKey& right) c } FRTConnectionPool::FRTConnectionPool(const ServerSpec & spec, const TimingValues & timingValues) - : _supervisor(std::make_unique<FRT_Supervisor>()), + : _threadPool(std::make_unique<FastOS_ThreadPool>(1024*60)), + _transport(std::make_unique<FNET_Transport>()), + _supervisor(std::make_unique<FRT_Supervisor>(_transport.get())), _selectIdx(0), _hostname("") { @@ -35,12 +38,12 @@ FRTConnectionPool::FRTConnectionPool(const ServerSpec & spec, const TimingValues _connections[key] = std::make_shared<FRTConnection>(spec.getHost(i), *_supervisor, timingValues); } setHostname(); - _supervisor->Start(); + _transport->Start(_threadPool.get()); } FRTConnectionPool::~FRTConnectionPool() { - _supervisor->ShutDown(true); + _transport->ShutDown(true); } void diff --git a/config/src/vespa/config/frt/frtconnectionpool.h b/config/src/vespa/config/frt/frtconnectionpool.h index e671c756db5..0b30e723272 100644 --- a/config/src/vespa/config/frt/frtconnectionpool.h +++ b/config/src/vespa/config/frt/frtconnectionpool.h @@ -7,6 +7,9 @@ #include <vector> #include <map> +class FNET_Transport; +class FastOS_ThreadPool; + namespace config { class FRTConnectionPool : public ConnectionFactory { @@ -29,6 +32,8 @@ private: int operator==(const FRTConnectionKey& right) const; }; + std::unique_ptr<FastOS_ThreadPool> _threadPool; + std::unique_ptr<FNET_Transport> _transport; std::unique_ptr<FRT_Supervisor> _supervisor; int _selectIdx; vespalib::string _hostname; |