Improve handling of exceptions during distributor startup

Remove call to requestShutdown which could try to use components
that weren't properly set up yet. Only used for _maybe_ being able
to scream an error to the cluster controller, but this has very
limited usefulness in practice.

Since exceptions are categorized and logged with backtrace at the
root application main level, remove redundant logging. Also enforce
component shutdown for network setup exceptions (not sure why this
wasn't there to start with).

3 files changed, 14 insertions, 22 deletions

diff --git a/storage/src/vespa/storage/storageserver/distributornode.cpp b/storage/src/vespa/storage/storageserver/distributornode.cpp
index 37174903697..fbbc7c366fd 100644
--- a/storage/src/vespa/storage/storageserver/distributornode.cpp
+++ b/storage/src/vespa/storage/storageserver/distributornode.cpp
@@ -23,9 +23,8 @@ DistributorNode::DistributorNode(
         StorageLink::UP communicationManager,
         std::unique_ptr<IStorageChainBuilder> storage_chain_builder)
     : StorageNode(configUri, context, generationFetcher,
-            std::unique_ptr<HostInfo>(new HostInfo()),
-                  communicationManager.get() == 0 ? NORMAL
-                                                  : SINGLE_THREADED_TEST_MODE),
+                  std::make_unique<HostInfo>(),
+                  !communicationManager ? NORMAL : SINGLE_THREADED_TEST_MODE),
       _threadPool(framework::TickingThreadPool::createDefault("distributor")),
       _context(context),
       _lastUniqueTimestampRequested(0),
@@ -36,16 +35,9 @@ DistributorNode::DistributorNode(
     if (storage_chain_builder) {
         set_storage_chain_builder(std::move(storage_chain_builder));
     }
-    try{
+    try {
         initialize();
-    } catch (const vespalib::NetworkSetupFailureException & e) {
-        LOG(warning, "Network failure: '%s'", e.what());
-        throw;
     } catch (const vespalib::Exception & e) {
-        LOG(error, "Caught exception %s during startup. Calling destruct "
-                   "functions in hopes of dying gracefully.",
-            e.getMessage().c_str());
-        requestShutdown("Failed to initialize: " + e.getMessage());
         shutdownDistributor();
         throw;
     }
diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index 74052932853..79205b8b513 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -369,13 +369,13 @@ StorageNode::removeConfigSubscriptions()
 void
 StorageNode::shutdown()
 {
-        // Try to shut down in opposite order of initialize. Bear in mind that
-        // we might be shutting down after init exception causing only parts
-        // of the server to have initialize
+    // Try to shut down in opposite order of initialize. Bear in mind that
+    // we might be shutting down after init exception causing only parts
+    // of the server to have initialize
     LOG(debug, "Shutting down storage node of type %s", getNodeType().toString().c_str());
     if (!_attemptedStopped) {
-        LOG(warning, "Storage killed before requestShutdown() was called. No "
-                     "reason has been given for why we're stopping.");
+        LOG(debug, "Storage killed before requestShutdown() was called. No "
+                   "reason has been given for why we're stopping.");
     }
         // Remove the subscription to avoid more callbacks from config
     removeConfigSubscriptions();
@@ -401,12 +401,12 @@ StorageNode::shutdown()
         _context.getComponentRegister().getMetricManager().stop();
     }
 
-        // Delete the status web server before the actual status providers, to
-        // ensure that web server does not query providers during shutdown
+    // Delete the status web server before the actual status providers, to
+    // ensure that web server does not query providers during shutdown
     _statusWebServer.reset();
 
-        // For this to be safe, noone can touch the state updater after we start
-        // deleting the storage chain
+    // For this to be safe, no-one can touch the state updater after we start
+    // deleting the storage chain
     LOG(debug, "Removing state updater pointer as we're about to delete it.");
     if (_chain) {
         LOG(debug, "Deleting storage chain");
diff --git a/storageserver/src/apps/storaged/storage.cpp b/storageserver/src/apps/storaged/storage.cpp
index 428067e3059..8fbeeb836da 100644
--- a/storageserver/src/apps/storaged/storage.cpp
+++ b/storageserver/src/apps/storaged/storage.cpp
@@ -187,7 +187,7 @@ int StorageApp::Main()
     // main loop - wait for termination signal
     while (!_process->getNode().attemptedStopped()) {
         if (_process->configUpdated()) {
-            LOG(debug, "Config updated. Progagating config updates");
+            LOG(debug, "Config updated. Propagating config updates");
             ResumeGuard guard(_process->getNode().pause());
             _process->updateConfig();
         }
@@ -197,7 +197,7 @@ int StorageApp::Main()
         handleSignals();
     }
     LOG(debug, "Server was attempted stopped, shutting down");
-    // Create guard that will forcifully kill storage if destruction takes longer
+    // Create guard that will forcefully kill storage if destruction takes longer
     // time than given timeout.
     vespalib::ShutdownGuard shutdownGuard(getMaxShutDownTime());
     LOG(debug, "Attempting proper shutdown");