incubator-blur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron McCurry (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (BLUR-277) Create an API to fetch the list of total Controller Servers in the Controller Cluster.
Date Sun, 13 Oct 2013 13:30:43 GMT

    [ https://issues.apache.org/jira/browse/BLUR-277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13793653#comment-13793653
] 

Aaron McCurry commented on BLUR-277:
------------------------------------

If at all possible it is best to avoid exceptions as logical execution paths, especially with
ZooKeeper.  While this code will work, the exception is logged in the ZooKeeper server logs
as well.  Overall this makes the logs noisy and hard to read.

Also since the registering of a controller is a one time thing (the PERSISTENT node) and because
it's only done at process startup, I think is would be better to check and create if missing.
 In most cases since controllers should have unique names (unless there is a config problem),
by checking and then creating an exception should never be thrown.

Also if there is a strange race condition between controller servers where they are both starting
at the same time and they are named the same thing (bad config).  Both could pass the looping
exists check and both go to register the EPHEMERAL nodes at the same time.  One wold create
the EPHEMERAL node and the other would throw an exception.  A NodeExistsException exception
and then that rogue process would continue on because it ate the exception and not blow up
like it should.  While I realize this is an edge case, but if it can happen it will likely
happen to someone and take a long time to debug the problem.

This is how I would rework the method:

  private void registerMyself() {
    String version = BlurUtil.getVersion();
    String controllerPath = ZookeeperPathConstants.getControllersPath() + "/" + _nodeName;
    try {
      if (_zookeeper.exists(controllerPath, false) == null) {
        // Don't set the version in the registered controllers, only in the online
        _zookeeper.create(controllerPath, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
      }
    } catch (KeeperException e) {
      throw new RuntimeException(e);
    } catch (InterruptedException e) {
      throw new RuntimeException(e);
    }

    try {
      String onlineControllerPath = ZookeeperPathConstants.getOnlineControllersPath() + "/"
+ _nodeName;
      while (_zookeeper.exists(onlineControllerPath, false) != null) {
        LOG.info("Node [{0}] already registered, waiting for path [{1}] to be released", _nodeName,
            onlineControllerPath);
        Thread.sleep(3000);
      }
      _zookeeper.create(onlineControllerPath, version.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.EPHEMERAL);
    } catch (KeeperException e) {
      throw new RuntimeException(e);
    } catch (InterruptedException e) {
      throw new RuntimeException(e);
    }
  }


Thanks!

Aaron

> Create an API to fetch the list of total Controller Servers in the Controller Cluster.
> --------------------------------------------------------------------------------------
>
>                 Key: BLUR-277
>                 URL: https://issues.apache.org/jira/browse/BLUR-277
>             Project: Apache Blur
>          Issue Type: Sub-task
>          Components: Blur
>    Affects Versions: 0.3.0
>            Reporter: Vikrant Navalgund
>             Fix For: 0.3.0
>
>         Attachments: BLUR-259-Subtask_1-MASTER.patch
>
>   Original Estimate: 6h
>  Remaining Estimate: 6h
>
> Right now we have an API to fetch the list of online controller servers. The method says
getControllerServerList() which in turn gets only the online list. 
> Suppose the Blur Shell needs a list of both the online and the offline controller list
we have no way of getting it today. This API gets the total controller list and with the existing
method to get the online controller list, the clients can do a Set difference and get the
offline list.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Mime
View raw message