Merge pull request #3336 from vespa-engine/balder/request-shutdown-instead-of-quick-exit-2

Balder/request shutdown instead of quick exit 2

8 files changed, 23 insertions, 37 deletions

diff --git a/searchcore/src/apps/proton/proton.cpp b/searchcore/src/apps/proton/proton.cpp
index 7515ddc4e23..5cdc8e04eb4 100644
--- a/searchcore/src/apps/proton/proton.cpp
+++ b/searchcore/src/apps/proton/proton.cpp
@@ -11,8 +11,6 @@
 #include <vespa/vespalib/io/fileutil.h>
 #include <vespa/config/common/exceptions.h>
 #include <vespa/fastos/app.h>
-
-#include <string>
 #include <iostream>
 
 #include <vespa/log/log.h>
@@ -224,9 +222,9 @@ App::Main()
                 proton.getMetricManager().init(params.identity, proton.getThreadPool());
             }
             EV_STARTED("proton");
-            while (!(SIG::INT.check() || SIG::TERM.check())) {
+            while (!(SIG::INT.check() || SIG::TERM.check() || (spiProton && spiProton->getNode().attemptedStopped()))) {
                 FastOS_Thread::Sleep(1000);
-                if (spiProton.get() && spiProton->configUpdated()) {
+                if (spiProton && spiProton->configUpdated()) {
                     storage::ResumeGuard guard(spiProton->getNode().pause());
                     spiProton->updateConfig();
                 }
@@ -247,7 +245,7 @@ App::Main()
                     }
                 }
             }
-            if (spiProton.get()) {
+            if (spiProton) {
                 spiProton->getNode().requestShutdown("controlled shutdown");
                 spiProton->shutdown();
                 EV_STOPPING("servicelayer", "clean shutdown");
diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
index bc13d8b31ab..483cffaf751 100644
--- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
+++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp
@@ -7,7 +7,6 @@
 #include <vespa/vespalib/util/host_name.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/component/vtag.h>
-#include <sstream>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".status");
@@ -57,7 +56,7 @@ void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::St
     int newPort = config->httpport;
         // If server is already running, ignore config updates that doesn't
         // alter port, or suggests random port.
-    if (_httpServer.get() != 0) {
+    if (_httpServer) {
         if (newPort == 0 || newPort == _port) return;
     }
         // Try to create new server before destroying old.
@@ -89,21 +88,23 @@ void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::St
         if (!started) {
             std::ostringstream ost;
             ost << "Failed to start status HTTP server using port " << newPort << ".";
-            if (_httpServer.get() != 0) {
+            if (_httpServer) {
                 ost << " Status server still running on port " << _port << " instead of suggested port " << newPort;
             }
-            LOG(fatal, "Failed to start status HTTP server using port %u. Old port was %u. Exiting.", newPort, _port);
-            std::quick_exit(67);
+            LOG(fatal, "%s.", ost.str().c_str());
+            _component->requestShutdown(ost.str());
+            _httpServer.reset();
+            return;
         }
             // Now that we know config update went well, update internal state
         _port = server->getListenPort();
         LOG(config, "Status pages now available on port %u", _port);
-        if (_httpServer.get() != 0) {
+        if (_httpServer) {
             LOG(debug, "Shutting down old status server.");
-            _httpServer.reset(0);
+            _httpServer.reset();
             LOG(debug, "Done shutting down old status server.");
         }
-    } else if (_httpServer.get() != 0) {
+    } else if (_httpServer) {
         LOG(info, "No longer running status server as negative port was given "
                   "in config, indicating not to run a server.");
     }
diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index 27f98b478ce..85f54431cfb 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -103,13 +103,13 @@ StorageNode::subscribeToConfigs()
 void
 StorageNode::initialize()
 {
+    _context.getComponentRegister().registerShutdownListener(*this);
+
     // Fetch configs needed first. These functions will just grab the config
     // and store them away, while having the config lock.
     subscribeToConfigs();
 
-    _context.getMemoryManager().setMaximumMemoryUsage(
-            _serverConfig->memorytouse);
-    _context.getComponentRegister().registerShutdownListener(*this);
+    _context.getMemoryManager().setMaximumMemoryUsage(_serverConfig->memorytouse);
     updateUpgradeFlag(*_clusterConfig);
 
     // First update some basics that doesn't depend on anything else to be
diff --git a/storage/src/vespa/storage/storageserver/storagenode.h b/storage/src/vespa/storage/storageserver/storagenode.h
index 4c4918e6977..753f3c85330 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.h
+++ b/storage/src/vespa/storage/storageserver/storagenode.h
@@ -111,7 +111,7 @@ private:
 
         // First components that doesn't depend on others
     std::unique_ptr<StatusWebServer>           _statusWebServer;
-    std::shared_ptr<StorageMetricSet>        _metrics;
+    std::shared_ptr<StorageMetricSet>          _metrics;
     std::unique_ptr<metrics::MetricManager>    _metricManager;
 
         // Depends on bucket databases and stop() functionality
@@ -122,8 +122,7 @@ private:
     std::unique_ptr<StatusMetricConsumer>      _statusMetrics;
         // Depends on metric manager
     std::unique_ptr<StateReporter>             _stateReporter;
-
-    std::unique_ptr<StateManager> _stateManager;
+    std::unique_ptr<StateManager>              _stateManager;
 
         // The storage chain can depend on anything.
     std::unique_ptr<StorageLink>               _chain;
diff --git a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
index cda2fee4628..a03f79b008d 100644
--- a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
+++ b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
@@ -5,9 +5,7 @@
 #include <vespa/metrics/metricmanager.h>
 #include <vespa/vespalib/util/exceptions.h>
 
-namespace storage {
-namespace framework {
-namespace defaultimplementation {
+namespace storage::framework::defaultimplementation {
 
 ComponentRegisterImpl::ComponentRegisterImpl()
     : _componentLock(),
@@ -184,6 +182,4 @@ ComponentRegisterImpl::registerShutdownListener(ShutdownListener& listener)
     _shutdownListener = &listener;
 }
 
-} // defaultimplementation
-} // framework
-} // storage
+}
diff --git a/storageframework/src/vespa/storageframework/generic/component/component.cpp b/storageframework/src/vespa/storageframework/generic/component/component.cpp
index 6457ed73795..a35cad68b00 100644
--- a/storageframework/src/vespa/storageframework/generic/component/component.cpp
+++ b/storageframework/src/vespa/storageframework/generic/component/component.cpp
@@ -4,10 +4,8 @@
 #include "componentregister.h"
 #include <vespa/storageframework/generic/metric/metricregistrator.h>
 #include <vespa/storageframework/generic/thread/threadpool.h>
-#include <vespa/vespalib/util/sync.h>
 
-namespace storage {
-namespace framework {
+namespace storage::framework {
 
 void
 Component::open()
@@ -137,6 +135,4 @@ Component::requestShutdown(vespalib::stringref reason)
     _componentRegister->requestShutdown(reason);
 }
 
-} // framework
-} // storage
-
+}
diff --git a/storageframework/src/vespa/storageframework/generic/component/componentregister.h b/storageframework/src/vespa/storageframework/generic/component/componentregister.h
index 521e23ba1aa..3babd8b29bc 100644
--- a/storageframework/src/vespa/storageframework/generic/component/componentregister.h
+++ b/storageframework/src/vespa/storageframework/generic/component/componentregister.h
@@ -13,8 +13,7 @@
 
 #include <vespa/vespalib/stllike/string.h>
 
-namespace storage {
-namespace framework {
+namespace storage::framework {
 
 class ManagedComponent;
 
@@ -25,6 +24,4 @@ struct ComponentRegister {
     virtual void requestShutdown(vespalib::stringref reason) = 0;
 };
 
-} // framework
-} // storage
-
+}
diff --git a/storageserver/src/vespa/storageserver/app/process.h b/storageserver/src/vespa/storageserver/app/process.h
index 030b51f929e..bbffca30593 100644
--- a/storageserver/src/vespa/storageserver/app/process.h
+++ b/storageserver/src/vespa/storageserver/app/process.h
@@ -19,7 +19,6 @@
 #include <vespa/config/subscription/configuri.h>
 #include <vespa/config/subscription/configsubscriber.h>
 
-
 namespace document { class DocumentTypeRepo; }
 
 namespace storage {