Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion conf/defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ storm.thrift.transport: "backtype.storm.security.auth.SimpleTransportPlugin"
storm.messaging.transport: "backtype.storm.messaging.netty.Context"

### nimbus.* configs are for the master
nimbus.host: "localhost"
nimbus.thrift.port: 6627

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if nimbus.host is not used any more we should either deprecate it or just remove it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nimbus.host relative config should be removed all over the source codes. I will fix this.

nimbus.thrift.max_buffer_size: 1048576
nimbus.childopts: "-Xmx1024m"
Expand Down
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.netflix.curator</groupId>
<artifactId>curator-recipes</artifactId>
<version>${curator.version}</version>
</dependency>
<dependency>
<groupId>com.googlecode.json-simple</groupId>
<artifactId>json-simple</artifactId>
Expand Down
4 changes: 4 additions & 0 deletions storm-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@
<groupId>com.netflix.curator</groupId>
<artifactId>curator-framework</artifactId>
</dependency>
<dependency>
<groupId>com.netflix.curator</groupId>
<artifactId>curator-recipes</artifactId>
</dependency>
<dependency>
<groupId>com.googlecode.json-simple</groupId>
<artifactId>json-simple</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions storm-core/src/clj/backtype/storm/config.clj
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@
(defn master-stormconf-path [stormroot]
(str stormroot file-path-separator "stormconf.ser"))

(defn master-tmp-dir [conf]
(let [ret (str (master-local-dir conf) file-path-separator "tmp")]
(FileUtils/forceMkdir (File. ret))
ret ))

(defn master-inbox [conf]
(let [ret (str (master-local-dir conf) file-path-separator "inbox")]
(FileUtils/forceMkdir (File. ret))
Expand Down
41 changes: 40 additions & 1 deletion storm-core/src/clj/backtype/storm/daemon/nimbus.clj
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
(:use [backtype.storm bootstrap util])
(:use [backtype.storm.config :only [validate-configs-with-schemas]])
(:use [backtype.storm.daemon common])
(:use [backtype.storm.nimbus leadership])
(:gen-class
:methods [^{:static true} [launch [backtype.storm.scheduler.INimbus] void]]))

Expand Down Expand Up @@ -894,10 +895,47 @@
)
)

(defn- sync-storm-code-from-leader [nimbus]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the standby nimbus has not downloaded everything when it becomes the master? The current code relies on the topology to be scheduled before the standby downloads anything. But there is a period of time between when the topology is submitted successfully and when the topology is scheduled where if nimbus goes down for whatever reason the topology will be killed. This is probably OK to start out with, but would be nice to fix.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case seems quite similar to nimbus hard-drive error in current version. They all make supervisors couldn't download topology codes from nimbus. Hoping storm-users might agree with us that it's acceptable to storm-users by re-submit the killed topology when this case happens.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that, it just would be nice to prevent it if at all possible. And perhaps when we move to bit-torrent for downloading that will take care of this too.

(let [conf (:conf nimbus)
storm-cluster-state (:storm-cluster-state nimbus)
storm-ids (.assignments storm-cluster-state nil)
storm-code-map (->> (dofor [sid storm-ids] {sid (.assignment-info storm-cluster-state sid nil)})
(apply merge)
(filter-val not-nil?)
(map-val :master-code-dir)
)
downloaded-storm-ids (set (map #(java.net.URLDecoder/decode %) (read-dir-contents (master-stormdist-root conf))))
tmproot (str (master-tmp-dir conf) file-path-separator (uuid))]
(doseq [[storm-id master-code-dir] storm-code-map]
(when (not (downloaded-storm-ids storm-id))
(log-message "Downloading code for storm id " storm-id " from " master-code-dir)

(FileUtils/forceMkdir (File. tmproot))
(Utils/downloadFromMaster conf (master-stormjar-path master-code-dir) (master-stormjar-path tmproot))
(Utils/downloadFromMaster conf (master-stormcode-path master-code-dir) (master-stormcode-path tmproot))
(Utils/downloadFromMaster conf (master-stormconf-path master-code-dir) (master-stormconf-path tmproot))
(FileUtils/moveDirectory (File. tmproot) (File. (master-stormdist-root conf storm-id)))

(log-message "Finished downloading code for storm id " storm-id " from " master-code-dir)
)
)
)
)

(defserverfn service-handler [conf inimbus]
(.prepare inimbus conf (master-inimbus-dir conf))
(log-message "Starting Nimbus with conf " conf)
(let [nimbus (nimbus-data conf inimbus)]
(let [nimbus (nimbus-data conf inimbus)
nimbus-leadership (nimbus-leadership conf)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because nimbus-leadership is opening a new connection to ZK is there ever a possibility that the nimbus-leadership connection will be lost (networking glitch) and the other ZK will not be? This could result in two nimbus instances both running at the same time.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About this i referred to the InterProcessMutex source code and believe that when networking glitch cause mutex.acquire() lose it's zk connection, an IOException will be throwed up by mutext.acquire() to cause this nimbus shutdown finally. Is there any other possibility may result in two or more nimbus instances?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern isn't in the acquire, it is after the acquire. The current code will open up two sockets to ZK. One is used for the mutex. The other is used for interaction with ZK. If the active nimbus, that has passed the mutex.acquire() already, now has a networking glitch that causes just the mutex connection to be dropped. I don't see how this will cause the currently active nimbus to get an IOException and shutdown.

;; Schedule synchronize storm code from leader
(schedule-recurring (:timer nimbus)
10
10
(fn []
(sync-storm-code-from-leader nimbus)
))
;; Compete to be nimbus leader
(acquire-leadership nimbus-leadership)
(.prepare ^backtype.storm.nimbus.ITopologyValidator (:validator nimbus) conf)
(cleanup-corrupt-topologies! nimbus)
(doseq [storm-id (.active-storms (:storm-cluster-state nimbus))]
Expand Down Expand Up @@ -1146,6 +1184,7 @@
(.disconnect (:storm-cluster-state nimbus))
(.cleanup (:downloaders nimbus))
(.cleanup (:uploaders nimbus))
(.close nimbus-leadership)
(log-message "Shut down master")
)
DaemonCommon
Expand Down
19 changes: 19 additions & 0 deletions storm-core/src/clj/backtype/storm/nimbus/leadership.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
(ns backtype.storm.nimbus.leadership

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an apache license header.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix this

(:import [backtype.storm.nimbus NimbusLeadership])
(:use [backtype.storm log]))

(defn nimbus-leadership [conf]
(NimbusLeadership. conf))

(defn get-nimbus-leader-address [conf]
(.getNimbusLeaderAddress (nimbus-leadership conf)))

(defn get-nimbus-hosts [conf]
(.getNimbusHosts (nimbus-leadership conf)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never call close on the nimbus-leadership. I believe that we are leaking a connection to ZK every time this this and get-nimbus-leader-address is called.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection to ZK is closed when nimbus shutdown. Considering the "nimbus-leadership" function is only called in "service-handler" in nimbus.clj to acquire leadership when nimbus launching, the relative codes are placed at the end of "service-handler" as "(.close nimbus-leadership)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it now you are right.


(defn acquire-leadership [nimbus-leadership]
(when-let [nimbus-leader-address (.getNimbusLeaderAddress nimbus-leadership)]
(log-message "Current Nimbus Leader: " nimbus-leader-address))
(log-message "acquiring nimbus leadership...")
(.acquireLeaderShip nimbus-leadership)
(log-message "acuqired nimbus leadership!"))
3 changes: 2 additions & 1 deletion storm-core/src/clj/backtype/storm/thrift.clj
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
(:import [org.apache.thrift.protocol TBinaryProtocol TProtocol])
(:import [org.apache.thrift.transport TTransport TFramedTransport TSocket])
(:use [backtype.storm util config log])
(:use [backtype.storm.nimbus leadership])
)

(defn instantiate-java-object [^JavaObject obj]
Expand Down Expand Up @@ -80,7 +81,7 @@

(defmacro with-configured-nimbus-connection [client-sym & body]
`(let [conf# (read-storm-config)
host# (conf# NIMBUS-HOST)
host# (.getHostName (get-nimbus-leader-address conf#))
port# (conf# NIMBUS-THRIFT-PORT)]
(with-nimbus-connection [~client-sym host# port#]
~@body )))
Expand Down
17 changes: 14 additions & 3 deletions storm-core/src/clj/backtype/storm/ui/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
(:use [backtype.storm config util log])
(:use [backtype.storm.ui helpers])
(:use [backtype.storm.daemon [common :only [ACKER-COMPONENT-ID system-id?]]])
(:use [backtype.storm.nimbus leadership])
(:use [ring.adapter.jetty :only [run-jetty]])
(:use [clojure.string :only [trim]])
(:import [backtype.storm.utils Utils])
Expand All @@ -39,7 +40,7 @@
(def ^:dynamic *STORM-CONF* (read-storm-config))

(defmacro with-nimbus [nimbus-sym & body]
`(thrift/with-nimbus-connection [~nimbus-sym (*STORM-CONF* NIMBUS-HOST) (*STORM-CONF* NIMBUS-THRIFT-PORT)]
`(thrift/with-nimbus-connection [~nimbus-sym (.getHostName (get-nimbus-leader-address *STORM-CONF*)) (*STORM-CONF* NIMBUS-THRIFT-PORT)]
~@body
))

Expand Down Expand Up @@ -155,8 +156,8 @@
(reduce +))]
(table [{:text "Version" :attr {:class "tip right"
:title (:version tips)}}
{:text "Nimbus uptime" :attr {:class "tip right"
:title (:nimbus-uptime tips)}}
{:text "Nimbus leader uptime" :attr {:class "tip right"
:title (:nimbus-leader-uptime tips)}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please actually define an entry in tips for ':nimbus-leader-uptime'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix the tips soon

{:text "Supervisors" :attr {:class "tip above"
:title (:num-supervisors tips)}}
{:text "Used slots" :attr {:class "tip above"
Expand All @@ -179,6 +180,15 @@
total-tasks]])
))

(defn nimbus-summary-table []
(let [nimbus-hosts (get-nimbus-hosts *STORM-CONF*)
nimbus-leader-host (get-nimbus-leader-address *STORM-CONF*)]
(table
["Nimbus address" "isLeader"]
(for [nimbus-host nimbus-hosts]
[(str (.getHostName nimbus-host) ":" (.getPort nimbus-host)) (if (= nimbus-host nimbus-leader-host) "true" "false")]
))))

(defn topology-link
([id] (topology-link id id))
([id content]
Expand Down Expand Up @@ -242,6 +252,7 @@
(let [summ (.getClusterInfo ^Nimbus$Client nimbus)]
(concat
[[:h2 "Cluster Summary"]]
[(nimbus-summary-table)]
[(cluster-summary-table summ)]
[[:h2 "Topology summary"]]
(main-topology-summary-table (.get_topologies summ))
Expand Down
104 changes: 104 additions & 0 deletions storm-core/src/jvm/backtype/storm/nimbus/NimbusLeadership.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package backtype.storm.nimbus;

import java.io.UnsupportedEncodingException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;

import backtype.storm.Config;
import backtype.storm.utils.Utils;

import com.netflix.curator.framework.CuratorFramework;
import com.netflix.curator.framework.recipes.locks.InterProcessMutex;
import com.netflix.curator.utils.ZKPaths;

@SuppressWarnings("rawtypes")
public class NimbusLeadership {

private static final String STORM_NIMBUS_LEADERSHIP_PATH = "/nimbus/leadership";

private Map conf;
private CuratorFramework curator;
private InterProcessMutex mutex;
private boolean isLeader = false;

public NimbusLeadership(final Map conf) {
this.conf = conf;
}

public void acquireLeaderShip() throws Exception {
String nimbusHostName = InetAddress.getLocalHost().getCanonicalHostName();
Object nimbusPort = conf.get(Config.NIMBUS_THRIFT_PORT);
String nodeId = nimbusHostName + ":" + nimbusPort.toString();
initCurator();
initLeadershipMutex(nodeId);
mutex.acquire();
isLeader = true;
}

public InetSocketAddress getNimbusLeaderAddress() throws Exception {
InetSocketAddress leaderAddress = null;
initCurator();
initLeadershipMutex(null);
Collection<String> nimbusNodesPath = mutex.getParticipantNodes();
if (nimbusNodesPath.size() > 0) {
leaderAddress = parseAddress(nimbusNodesPath.iterator().next());
}
close();
return leaderAddress;
}

public List<InetSocketAddress> getNimbusHosts() throws Exception {
List<InetSocketAddress> nimbusAddressList = new ArrayList<InetSocketAddress>();
initCurator();
initLeadershipMutex(null);
Collection<String> nimbusNodesPath = mutex.getParticipantNodes();
for (String nimbusNodePath : nimbusNodesPath) {
nimbusAddressList.add(parseAddress(nimbusNodePath));
}
close();
return nimbusAddressList;
}

public void close() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spacing appears to be off here compared to the rest of the file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invisible characters here are spaces which should be replaced by tabs. There are several pieces of codes in other files have the same problem. All will be fixed.

if (isLeader) {
try {
mutex.release();
} catch (Exception e) {
throw new RuntimeException("Exception while releasing mutex", e);
}
}
curator.close();
}

@SuppressWarnings("unchecked")
private void initCurator() throws Exception {
List<String> servers = (List<String>) conf.get(Config.STORM_ZOOKEEPER_SERVERS);
Object port = conf.get(Config.STORM_ZOOKEEPER_PORT);
this.curator = Utils.newCuratorStarted(conf, servers, port);
}

private void initLeadershipMutex(final String nodeId) throws Exception {
String path = (String)conf.get(Config.STORM_ZOOKEEPER_ROOT) + STORM_NIMBUS_LEADERSHIP_PATH;
ZKPaths.mkdirs(curator.getZookeeperClient().getZooKeeper(), path);
mutex = new InterProcessMutex(curator, path) {
@Override
protected byte[] getLockNodeBytes() {
try {
return nodeId == null ? null : nodeId.getBytes("UTF-8");
} catch (UnsupportedEncodingException e) {
throw new RuntimeException("UTF-8 isn't supported", e);
}
}
};
}

private InetSocketAddress parseAddress(String nimbusNodePath) throws Exception {
String nimbusNodeData = new String(curator.getData().forPath(nimbusNodePath), "UTF-8");
String[] split = nimbusNodeData.split(":");
return new InetSocketAddress(split[0], Integer.parseInt(split[1]));
}
}
8 changes: 6 additions & 2 deletions storm-core/src/jvm/backtype/storm/utils/NimbusClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,23 @@
import org.apache.thrift.transport.TTransportException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import backtype.storm.nimbus.NimbusLeadership;

public class NimbusClient extends ThriftClient {
private Nimbus.Client _client;
private static final Logger LOG = LoggerFactory.getLogger(NimbusClient.class);

public static NimbusClient getConfiguredClient(Map conf) {
try {
String nimbusHost = (String) conf.get(Config.NIMBUS_HOST);
NimbusLeadership nimbusLeadership = new NimbusLeadership(conf);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leadership is never closed here either.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection to ZK here is closed in next line "nimbusLeadership.getNimbusLeaderAddress()". The "getNimbusLeaderAddress()" function has finally closed CuratorFramework calling "close();"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that now. I messed the calls to close() in getNimbusLeaderAddress() and getNimbusHosts(). That seems counter intuitive to me but it works.

String nimbusHost = nimbusLeadership.getNimbusLeaderAddress().getHostName();
int nimbusPort = Utils.getInt(conf.get(Config.NIMBUS_THRIFT_PORT));
return new NimbusClient(conf, nimbusHost, nimbusPort);
} catch (TTransportException ex) {
throw new RuntimeException(ex);
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}

public NimbusClient(Map conf, String host, int port) throws TTransportException {
Expand Down