From 7427ac206cc66baecc501dfd70b68f2d3a101651 Mon Sep 17 00:00:00 2001
From: Shay Banon <kimchy@gmail.com>
Date: Mon, 13 Jan 2014 12:26:53 -0800
Subject: [PATCH] apply comments on tribe code

---
 .../elasticsearch/cluster/ClusterService.java |  3 ++
 .../common/settings/ImmutableSettings.java    |  3 ++
 .../node/internal/InternalNode.java           |  1 +
 .../org/elasticsearch/tribe/TribeService.java | 32 ++++++++++++++++---
 4 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/main/java/org/elasticsearch/cluster/ClusterService.java b/src/main/java/org/elasticsearch/cluster/ClusterService.java
index bdef076d0be..6204599f57d 100644
--- a/src/main/java/org/elasticsearch/cluster/ClusterService.java
+++ b/src/main/java/org/elasticsearch/cluster/ClusterService.java
@@ -51,6 +51,9 @@ public interface ClusterService extends LifecycleComponent<ClusterService> {
      */
     void addInitialStateBlock(ClusterBlock block) throws ElasticsearchIllegalStateException;
 
+    /**
+     * Remove an initial block to be set on the first cluster state created.
+     */
     void removeInitialStateBlock(ClusterBlock block) throws ElasticsearchIllegalStateException;
 
     /**
diff --git a/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java b/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java
index a88710a188f..e3572931876 100644
--- a/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java
+++ b/src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java
@@ -497,6 +497,9 @@ public class ImmutableSettings implements Settings {
 
     @Override
     public Map<String, Settings> getGroups(String settingPrefix, boolean ignoreNonGrouped) throws SettingsException {
+        if (!Strings.hasLength(settingPrefix)) {
+            throw new ElasticsearchIllegalArgumentException("illegal setting prefix " + settingPrefix);
+        }
         if (settingPrefix.charAt(settingPrefix.length() - 1) != '.') {
             settingPrefix = settingPrefix + ".";
         }
diff --git a/src/main/java/org/elasticsearch/node/internal/InternalNode.java b/src/main/java/org/elasticsearch/node/internal/InternalNode.java
index 33e5f336c95..f9e8ae6269f 100644
--- a/src/main/java/org/elasticsearch/node/internal/InternalNode.java
+++ b/src/main/java/org/elasticsearch/node/internal/InternalNode.java
@@ -142,6 +142,7 @@ public final class InternalNode implements Node {
 
         this.pluginsService = new PluginsService(tuple.v1(), tuple.v2());
         this.settings = pluginsService.updatedSettings();
+        // create the environment based on the finalized (processed) view of the settings
         this.environment = new Environment(this.settings());
 
         CompressorFactory.configure(settings);
diff --git a/src/main/java/org/elasticsearch/tribe/TribeService.java b/src/main/java/org/elasticsearch/tribe/TribeService.java
index 810e03de44b..f28224ef96b 100644
--- a/src/main/java/org/elasticsearch/tribe/TribeService.java
+++ b/src/main/java/org/elasticsearch/tribe/TribeService.java
@@ -22,6 +22,7 @@ package org.elasticsearch.tribe;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.ElasticsearchInterruptedException;
 import org.elasticsearch.cluster.*;
 import org.elasticsearch.cluster.block.ClusterBlock;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
@@ -161,24 +162,47 @@ public class TribeService extends AbstractLifecycleComponent<TribeService> {
         try {
             latch.await();
         } catch (InterruptedException e) {
-            // ignore
+            throw new ElasticsearchInterruptedException(e.getMessage(), e);
         }
         for (InternalNode node : nodes) {
-            node.start();
+            try {
+                node.start();
+            } catch (Throwable e) {
+                // calling close is safe for non started nodes, we can just iterate over all
+                for (InternalNode otherNode : nodes) {
+                    try {
+                        otherNode.close();
+                    } catch (Throwable t) {
+                        logger.warn("failed to close node {} on failed start", otherNode, t);
+                    }
+                }
+                if (e instanceof RuntimeException) {
+                    throw (RuntimeException) e;
+                }
+                throw new ElasticsearchException(e.getMessage(), e);
+            }
         }
     }
 
     @Override
     protected void doStop() throws ElasticsearchException {
         for (InternalNode node : nodes) {
-            node.stop();
+            try {
+                node.stop();
+            } catch (Throwable t) {
+                logger.warn("failed to stop node {}", t, node);
+            }
         }
     }
 
     @Override
     protected void doClose() throws ElasticsearchException {
         for (InternalNode node : nodes) {
-            node.close();
+            try {
+                node.close();
+            } catch (Throwable t) {
+                logger.warn("failed to close node {}", t, node);
+            }
         }
     }