From 1a217ade5f61adb837e9f0a257b92a63ba47a2ae Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 5 Apr 2018 19:34:49 +0200 Subject: Revert "Balder/quick restart of slobrok" --- .../java/com/yahoo/vespa/model/admin/Slobrok.java | 18 +++---- .../com/yahoo/vespa/model/admin/AdminTestCase.java | 8 --- config/src/vespa/config/helper/configfetcher.cpp | 2 +- configdefinitions/src/vespa/CMakeLists.txt | 2 - configdefinitions/src/vespa/stateserver.def | 5 -- .../src/test/apps/app-jdisc-only-restart/hosts.xml | 7 --- .../searchdefinitions/music.sd | 57 -------------------- .../test/apps/app-jdisc-only-restart/services.xml | 29 ---------- .../config/server/ApplicationRepositoryTest.java | 4 +- slobrok/src/apps/slobrok/slobrok.cpp | 11 ++-- slobrok/src/vespa/slobrok/cfg.cpp | 3 ++ slobrok/src/vespa/slobrok/server/CMakeLists.txt | 1 - slobrok/src/vespa/slobrok/server/configshim.cpp | 12 ++--- slobrok/src/vespa/slobrok/server/configshim.h | 11 ++-- .../slobrok/server/reconfigurable_stateserver.cpp | 61 ---------------------- .../slobrok/server/reconfigurable_stateserver.h | 31 ----------- slobrok/src/vespa/slobrok/server/sbenv.cpp | 42 +++++++-------- slobrok/src/vespa/slobrok/server/sbenv.h | 6 ++- .../src/vespa/vespalib/net/state_server.cpp | 2 +- 19 files changed, 52 insertions(+), 260 deletions(-) delete mode 100644 configdefinitions/src/vespa/stateserver.def delete mode 100644 configserver/src/test/apps/app-jdisc-only-restart/hosts.xml delete mode 100644 configserver/src/test/apps/app-jdisc-only-restart/searchdefinitions/music.sd delete mode 100644 configserver/src/test/apps/app-jdisc-only-restart/services.xml delete mode 100644 slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.cpp delete mode 100644 slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.h diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java index aa6192291d3..093694f41b3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.model.admin; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.vespa.config.StateserverConfig; import com.yahoo.vespa.model.AbstractService; /** @@ -10,14 +9,9 @@ import com.yahoo.vespa.model.AbstractService; * * @author gjoranv */ -public class Slobrok extends AbstractService implements StateserverConfig.Producer { +public class Slobrok extends AbstractService { private static final long serialVersionUID = 1L; - @Override - public void getConfig(StateserverConfig.Builder builder) { - builder.httpport(getStatePort()); - } - /** * @param parent The parent ConfigProducer. * @param index unique index for all slobroks @@ -46,7 +40,9 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc } public String getStartupCommand() { - return "exec $ROOT/sbin/vespa-slobrok -p " + getPort() + " -c " + getConfigId(); + return "exec $ROOT/sbin/vespa-slobrok -p " + getPort() + + " -s " + getStatePort() + + " -c " + getConfigId(); } /** @@ -59,15 +55,15 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc /** * @return The port on which this slobrok should respond, as a String. */ - private String getPort() { + public String getPort() { return String.valueOf(getRelativePort(0)); } /** * @return The port on which the state server should respond */ - public int getStatePort() { - return getRelativePort(1); + public String getStatePort() { + return String.valueOf(getRelativePort(1)); } /** diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java index a88d3b388a9..5e6241a7043 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java @@ -17,7 +17,6 @@ import com.yahoo.config.provision.Zone; import com.yahoo.container.StatisticsConfig; import com.yahoo.container.jdisc.config.HealthMonitorConfig; import com.yahoo.net.HostName; -import com.yahoo.vespa.config.StateserverConfig; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.component.Component; @@ -84,13 +83,6 @@ public class AdminTestCase { } assertTrue(localHostOK); - StateserverConfig.Builder ssb = new StateserverConfig.Builder(); - vespaModel.getConfig(ssb, "admin/slobrok.0"); - assertEquals(19100, new StateserverConfig(ssb).httpport()); - - vespaModel.getConfig(ssb, "admin/slobrok.1"); - assertEquals(19102, new StateserverConfig(ssb).httpport()); - LogdConfig.Builder lb = new LogdConfig.Builder(); vespaModel.getConfig(lb, "admin/slobrok.0"); LogdConfig lc = new LogdConfig(lb); diff --git a/config/src/vespa/config/helper/configfetcher.cpp b/config/src/vespa/config/helper/configfetcher.cpp index 5a3cdad814e..b7e15240255 100644 --- a/config/src/vespa/config/helper/configfetcher.cpp +++ b/config/src/vespa/config/helper/configfetcher.cpp @@ -16,7 +16,7 @@ ConfigFetcher::ConfigFetcher(const IConfigContext::SP & context) } ConfigFetcher::ConfigFetcher(const SourceSpec & spec) - : _poller(std::make_shared(spec)), + : _poller(IConfigContext::SP(new ConfigContext(spec))), _thread(std::make_unique(_poller)), _closed(false), _started(false) diff --git a/configdefinitions/src/vespa/CMakeLists.txt b/configdefinitions/src/vespa/CMakeLists.txt index d7023e19622..e1e7a5a0c18 100644 --- a/configdefinitions/src/vespa/CMakeLists.txt +++ b/configdefinitions/src/vespa/CMakeLists.txt @@ -74,5 +74,3 @@ vespa_generate_config(configdefinitions zookeepers.def) install_config_definition(zookeepers.def cloud.config.zookeepers.def) vespa_generate_config(configdefinitions bucketspaces.def) install_config_definition(bucketspaces.def vespa.config.content.core.bucketspaces.def) -vespa_generate_config(configdefinitions stateserver.def) -install_config_definition(stateserver.def vespa.config.stateserver.def) diff --git a/configdefinitions/src/vespa/stateserver.def b/configdefinitions/src/vespa/stateserver.def deleted file mode 100644 index 5be221efb49..00000000000 --- a/configdefinitions/src/vespa/stateserver.def +++ /dev/null @@ -1,5 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -namespace=vespa.config - -# Port used for serving the state/vN api -httpport int default = 0 diff --git a/configserver/src/test/apps/app-jdisc-only-restart/hosts.xml b/configserver/src/test/apps/app-jdisc-only-restart/hosts.xml deleted file mode 100644 index f4256c9fc81..00000000000 --- a/configserver/src/test/apps/app-jdisc-only-restart/hosts.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - - - node1 - - diff --git a/configserver/src/test/apps/app-jdisc-only-restart/searchdefinitions/music.sd b/configserver/src/test/apps/app-jdisc-only-restart/searchdefinitions/music.sd deleted file mode 100644 index 2e40523a6d9..00000000000 --- a/configserver/src/test/apps/app-jdisc-only-restart/searchdefinitions/music.sd +++ /dev/null @@ -1,57 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# A basic search definition - called music, should be saved to music.sd -search music { - - # It contains one document type only - called music as well - document music { - - field title type string { - indexing: summary | index # How this field should be indexed - # index-to: title, default # Create two indexes - weight: 75 # Ranking importancy of this field, used by the built in nativeRank feature - header - } - - field artist type string { - indexing: summary | attribute | index - # index-to: artist, default - - weight: 25 - header - } - - field year type int { - indexing: summary | attribute - header - } - - # Increase query - field popularity type int { - indexing: summary | attribute - body - } - - field url type uri { - indexing: summary | index - header - } - - } - - rank-profile default inherits default { - first-phase { - expression: nativeRank(title,artist) + attribute(popularity) - } - - } - - rank-profile textmatch inherits default { - first-phase { - expression: nativeRank(title,artist) - } - - } - - - -} diff --git a/configserver/src/test/apps/app-jdisc-only-restart/services.xml b/configserver/src/test/apps/app-jdisc-only-restart/services.xml deleted file mode 100644 index b864ea206ef..00000000000 --- a/configserver/src/test/apps/app-jdisc-only-restart/services.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - - - - - - - - - - - - - - - - - - someval - - - - - - - - - diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index 0b7e9a78967..ecc3a8a0a54 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -33,8 +33,6 @@ public class ApplicationRepositoryTest { private final static File testApp = new File("src/test/apps/app"); private final static File testAppJdiscOnly = new File("src/test/apps/app-jdisc-only"); - private final static File testAppJdiscOnlyRestart = new File("src/test/apps/app-jdisc-only-restart"); - private final static TenantName tenantName = TenantName.from("test"); private final static Clock clock = Clock.systemUTC(); @@ -65,7 +63,7 @@ public class ApplicationRepositoryTest { @Test public void prepareAndActivateWithRestart() throws IOException { prepareAndActivateApp(testAppJdiscOnly); - PrepareResult result = prepareAndActivateApp(testAppJdiscOnlyRestart); + PrepareResult result = prepareAndActivateApp(testApp); assertTrue(result.configChangeActions().getRefeedActions().isEmpty()); assertFalse(result.configChangeActions().getRestartActions().isEmpty()); } diff --git a/slobrok/src/apps/slobrok/slobrok.cpp b/slobrok/src/apps/slobrok/slobrok.cpp index e69f2df53f0..277cc2f3a87 100644 --- a/slobrok/src/apps/slobrok/slobrok.cpp +++ b/slobrok/src/apps/slobrok/slobrok.cpp @@ -50,6 +50,7 @@ int App::Main() { uint32_t portnum = 2773; + uint32_t statePort = 0; vespalib::string cfgId; int argi = 1; @@ -60,6 +61,9 @@ App::Main() case 'c': cfgId = std::string(optArg); break; + case 's': + statePort = atoi(optArg); + break; case 'p': portnum = atoi(optArg); break; @@ -73,11 +77,10 @@ App::Main() if (cfgId.empty()) { LOG(debug, "no config id specified"); ConfigShim shim(portnum); - mainobj = std::make_unique(shim); + mainobj.reset(new SBEnv(shim)); } else { - ConfigShim shim(portnum, cfgId); - shim.enableStateServer(true); - mainobj = std::make_unique(shim); + ConfigShim shim(portnum, statePort, cfgId); + mainobj.reset(new SBEnv(shim)); } hook_sigterm(); res = mainobj->MainLoop(); diff --git a/slobrok/src/vespa/slobrok/cfg.cpp b/slobrok/src/vespa/slobrok/cfg.cpp index a1f165fa7ea..720fb5df962 100644 --- a/slobrok/src/vespa/slobrok/cfg.cpp +++ b/slobrok/src/vespa/slobrok/cfg.cpp @@ -1,6 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "cfg.h" +#include +LOG_SETUP(".slobrok.configurator"); + namespace slobrok { namespace { diff --git a/slobrok/src/vespa/slobrok/server/CMakeLists.txt b/slobrok/src/vespa/slobrok/server/CMakeLists.txt index a6772404ead..5ca41967524 100644 --- a/slobrok/src/vespa/slobrok/server/CMakeLists.txt +++ b/slobrok/src/vespa/slobrok/server/CMakeLists.txt @@ -22,7 +22,6 @@ vespa_add_library(slobrok_slobrokserver slobrokserver.cpp visible_map.cpp metrics_producer.cpp - reconfigurable_stateserver.cpp INSTALL lib64 DEPENDS slobrok diff --git a/slobrok/src/vespa/slobrok/server/configshim.cpp b/slobrok/src/vespa/slobrok/server/configshim.cpp index 15c39516ded..83f91a29b30 100644 --- a/slobrok/src/vespa/slobrok/server/configshim.cpp +++ b/slobrok/src/vespa/slobrok/server/configshim.cpp @@ -5,26 +5,24 @@ namespace slobrok { ConfigShim::ConfigShim(uint32_t port) - : _port(port), - _enableStateServer(false), - _configId(""), + : _port(port), _statePort(0), _configId(""), _factory(config::ConfigUri::createEmpty()) {} -ConfigShim::ConfigShim(uint32_t port, const std::string& cfgId) +ConfigShim::ConfigShim(uint32_t port, uint32_t statePort_in, const std::string& cfgId) : _port(port), - _enableStateServer(false), + _statePort(statePort_in), _configId(cfgId), _factory(config::ConfigUri(_configId)) {} ConfigShim::ConfigShim(uint32_t port, const std::string& cfgId, config::IConfigContext::SP cfgCtx) : _port(port), - _enableStateServer(false), + _statePort(0), _configId(cfgId), _factory(config::ConfigUri(cfgId, cfgCtx)) {} -ConfigShim::~ConfigShim() = default; +ConfigShim::~ConfigShim() {} } diff --git a/slobrok/src/vespa/slobrok/server/configshim.h b/slobrok/src/vespa/slobrok/server/configshim.h index 750f6a4c139..ed3c04b6233 100644 --- a/slobrok/src/vespa/slobrok/server/configshim.h +++ b/slobrok/src/vespa/slobrok/server/configshim.h @@ -9,20 +9,19 @@ namespace slobrok { class ConfigShim { private: - uint32_t _port; - bool _enableStateServer; - std::string _configId; + uint32_t _port; + uint32_t _statePort; + std::string _configId; ConfiguratorFactory _factory; public: ConfigShim(uint32_t port); - ConfigShim(uint32_t port, const std::string& cfgId); + ConfigShim(uint32_t port, uint32_t statePort_in, const std::string& cfgId); ConfigShim(uint32_t port, const std::string& cfgId, config::IConfigContext::SP cfgCtx); ~ConfigShim(); - ConfigShim & enableStateServer(bool v) { _enableStateServer = v; return *this; } - bool enableStateServer() const { return _enableStateServer; } uint32_t portNumber() const { return _port; } + uint32_t statePort() const { return _statePort; } std::string configId() const { return _configId; } const char *id() const { return _configId.c_str(); } const ConfiguratorFactory & factory() const { return _factory; } diff --git a/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.cpp b/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.cpp deleted file mode 100644 index ca6dcad1932..00000000000 --- a/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.cpp +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "reconfigurable_stateserver.h" -#include -#include -#include - -#include -#include - -LOG_SETUP(".reconfigurable_stateserver"); - -using namespace std::chrono_literals; - -namespace slobrok { - -ReconfigurableStateServer::ReconfigurableStateServer(const config::ConfigUri & configUri, - vespalib::HealthProducer & health, - vespalib::MetricsProducer & metrics, - vespalib::ComponentConfigProducer & components) - : _health(health), - _metrics(metrics), - _components(components), - _configFetcher(std::make_unique(configUri.getContext())), - _server() -{ - _configFetcher->subscribe(configUri.getConfigId(), this); - _configFetcher->start(); -} - -ReconfigurableStateServer::~ReconfigurableStateServer() -{ - _configFetcher->close(); -} - -void -ReconfigurableStateServer::configure(std::unique_ptr config) -{ - _server.reset(); - for (size_t retryTime(1); !_server && (retryTime < 10); retryTime++) { - try { - _server = std::make_unique(config->httpport, _health, _metrics, _components); - } catch (vespalib::PortListenException & e) { - LOG(warning, "Failed listening to network port(%d) with protocol(%s): '%s', will retry for 60s", - e.get_port(), e.get_protocol().c_str(), e.what()); - std::this_thread::sleep_for(retryTime * 1s); - } - } - if (!_server) { - try { - _server = std::make_unique(config->httpport, _health, _metrics, _components); - } catch (vespalib::PortListenException & e) { - LOG(error, "Failed listening to network port(%d) with protocol(%s): '%s', giving up and restarting.", - e.get_port(), e.get_protocol().c_str(), e.what()); - std::quick_exit(17); - } - } - -} - -} diff --git a/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.h b/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.h deleted file mode 100644 index a8747d09bfd..00000000000 --- a/slobrok/src/vespa/slobrok/server/reconfigurable_stateserver.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include -#include -#include - -namespace vespalib { - class HealthProducer; - class MetricsProducer; - class ComponentConfigProducer; - class StateServer; -} -namespace slobrok { - -class ReconfigurableStateServer : private config::IFetcherCallback { -public: - ReconfigurableStateServer(const config::ConfigUri & configUri, - vespalib::HealthProducer & healt, - vespalib::MetricsProducer & metrics, - vespalib::ComponentConfigProducer & component); - ~ReconfigurableStateServer(); -private: - void configure(std::unique_ptr config) override; - vespalib::HealthProducer & _health; - vespalib::MetricsProducer & _metrics; - vespalib::ComponentConfigProducer & _components; - std::unique_ptr _configFetcher; - std::unique_ptr _server; -}; - -} diff --git a/slobrok/src/vespa/slobrok/server/sbenv.cpp b/slobrok/src/vespa/slobrok/server/sbenv.cpp index 35af0daf24a..002dd2244ca 100644 --- a/slobrok/src/vespa/slobrok/server/sbenv.cpp +++ b/slobrok/src/vespa/slobrok/server/sbenv.cpp @@ -1,25 +1,20 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "reconfigurable_stateserver.h" #include "sbenv.h" #include "selfcheck.h" #include "remote_check.h" +#include +#include #include -#include +#include #include #include #include -#include -#include -#include #include LOG_SETUP(".sbenv"); -using namespace std::chrono_literals; - namespace slobrok { - namespace { void @@ -85,16 +80,17 @@ ConfigTask::PerformTask() } // namespace slobrok:: SBEnv::SBEnv(const ConfigShim &shim) - : _transport(std::make_unique()), - _supervisor(std::make_unique(_transport.get(), nullptr)), - _configShim(shim), + : _transport(new FNET_Transport()), + _supervisor(new FRT_Supervisor(_transport.get(), NULL)), + _sbPort(shim.portNumber()), + _statePort(shim.statePort()), _configurator(shim.factory().create(*this)), _shuttingDown(false), _partnerList(), _me(), _rpcHooks(*this, _rpcsrvmap, _rpcsrvmanager), - _selfchecktask(std::make_unique(getSupervisor()->GetScheduler(), _rpcsrvmap, _rpcsrvmanager)), - _remotechecktask(std::make_unique(getSupervisor()->GetScheduler(), _rpcsrvmap, _rpcsrvmanager, _exchanger)), + _selfchecktask(new SelfCheck(getSupervisor()->GetScheduler(), _rpcsrvmap, _rpcsrvmanager)), + _remotechecktask(new RemoteCheck(getSupervisor()->GetScheduler(), _rpcsrvmap, _rpcsrvmanager, _exchanger)), _health(), _metrics(_rpcHooks, *_transport), _components(), @@ -102,7 +98,7 @@ SBEnv::SBEnv(const ConfigShim &shim) _exchanger(*this, _rpcsrvmap), _rpcsrvmap() { - srandom(time(nullptr) ^ getpid()); + srandom(time(NULL) ^ getpid()); _rpcHooks.initRPC(getSupervisor()); } @@ -162,22 +158,20 @@ toString(const std::vector & v) { int SBEnv::MainLoop() { - if (! getSupervisor()->Listen(_configShim.portNumber())) { - LOG(error, "unable to listen to port %d", _configShim.portNumber()); + vespalib::StateServer stateServer(_statePort, _health, _metrics, _components); + + if (! getSupervisor()->Listen(_sbPort)) { + LOG(error, "unable to listen to port %d", _sbPort); EV_STOPPING("slobrok", "could not listen"); return 1; } else { - LOG(config, "listening on port %d", _configShim.portNumber()); + LOG(config, "listening on port %d", _sbPort); } - std::string myspec = createSpec(_configShim.portNumber()); - - _me = std::make_unique(myspec.c_str(), myspec.c_str(), _rpcsrvmanager); + std::string myspec = createSpec(_sbPort); - std::unique_ptr stateServer; - if (_configShim.enableStateServer()) { - stateServer = std::make_unique(_configShim.configId(), _health, _metrics, _components); - } + _me.reset(new ManagedRpcServer(myspec.c_str(), myspec.c_str(), + _rpcsrvmanager)); try { _configurator->poll(); diff --git a/slobrok/src/vespa/slobrok/server/sbenv.h b/slobrok/src/vespa/slobrok/server/sbenv.h index 8c897c9c999..f96b7540b3a 100644 --- a/slobrok/src/vespa/slobrok/server/sbenv.h +++ b/slobrok/src/vespa/slobrok/server/sbenv.h @@ -8,10 +8,10 @@ #include "exchange_manager.h" #include "configshim.h" #include "ok_state.h" -#include "metrics_producer.h" #include #include #include +#include "metrics_producer.h" #include class FastOS_ThreadPool; @@ -23,6 +23,7 @@ namespace slobrok { class NamedService; class ManagedRpcServer; +class RemoteRpcServer; class RPCHooks; class SelfCheck; class RemoteCheck; @@ -39,7 +40,8 @@ private: std::unique_ptr _transport; std::unique_ptr _supervisor; - ConfigShim _configShim; + uint32_t _sbPort; + uint32_t _statePort; Configurator::UP _configurator; bool _shuttingDown; diff --git a/staging_vespalib/src/vespa/vespalib/net/state_server.cpp b/staging_vespalib/src/vespa/vespalib/net/state_server.cpp index bb81a3aa535..61c3e4e6f74 100644 --- a/staging_vespalib/src/vespa/vespalib/net/state_server.cpp +++ b/staging_vespalib/src/vespa/vespalib/net/state_server.cpp @@ -17,6 +17,6 @@ StateServer::StateServer(int port, _server.start(); } -StateServer::~StateServer() = default; +StateServer::~StateServer() {} } // namespace vespalib -- cgit v1.2.3