[jira] [Issue Comment Edited] (HAMA-456) Add getPeerName(int index) and Fix getAllPeerNames()

["Thomas Jungblut (Issue Comment Edited) (JIRA)" <ji...@apache.org>]

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

Thomas Jungblut edited comment on HAMA-456 at 10/18/11 12:19 PM:
-----------------------------------------------------------------

+1, please commit this.


I think we should directly split this into the refactoring of BSPPeerImpl and refactor a Barrier Sync Part.

BarrierSync should consist of two major parts, server and client. In HAMA the server part must be launched as an additional daemon at startup time (e.G. Zookeeper), in YARN this will be launched as a seperate thread inside the application master. 

So server interface must contain following two methods:

{noformat}
// In YARN port and hostname of the sync server is only known at runtime, 
// so this method should modify the conf to set the host:port of the syncserver that has been started.
public Configuration start(Configuration conf);
public void stop();
{noformat}

Clientside has some methods in common which are already used by the RPC implementation 

{noformat}

public void init(Configuration conf);

public void enterBarrier();

public void leaveBarrier();

// ... what is also needed.

{noformat}

BSPPeer then contains a factory which will just look at the configuration which client class has been configured (default class is ZooKeeper implementation) and then reflectively instantiate a client instance. Then it calls "init" with the configuration where the host:port data resides.
This should seal the deal!

Afterwards we can refator the BSPPeerImpl to use the new stuff. This will reduce LOC and drastically reduce the complexity.
In BSPPeerImpl we should look at code which is useless, for example the "throws NullPointerException" code which does never return null although it catches it and just logs an error.
IMO all the serializer classes should be declared as a seperate java file. They mess up the code too.

Anything to add?
                
      was (Author: thomas.jungblut):
    +1, please commit this.


I think we should directly split this into the refactoring of BSPPeerImpl and refactor a Barrier Sync Part.

BarrierSync should consist of two major parts, server and client. In HAMA the server part must be launched as an additional daemon at startup time (e.G. Zookeeper), in YARN this will be launched as a seperate thread inside the application master. 

So server interface must contain following two methods:

{noformat}
// In YARN port and hostname of the sync server is only known at runtime, 
// so this method should modify the conf to set the host:port of the syncserver that has been started.
public Configuration start(Configuration conf);
public void stop();
{noformat}

Clientside has some methods in common which are already used by the RPC implementation 

{noformat}

public void init(Configuration conf);

public void enterBarrier();

public void leaveBarrier();

// ... what is also needed.

{noformat}

BSPPeer then contains a factory which will just look at the configuration which client class has been configured (default class is ZooKeeper implementation) and then reflectively instantiate a client instance. Then it calls "init" with the configuration where the host:port data resides.
This should seal the deal!

Afterwards we can refator the BSPPeerImpl to use the new stuff. This will reduce LOC and drastically reduce the complexity.
In BSPPeerImpl we should look at code which is useless, for example the "throws NullPointerException" code which does never return null although it catches it and just logs an error.

Anything to add?
                  
> Add getPeerName(int index) and Fix getAllPeerNames()
> ----------------------------------------------------
>
>                 Key: HAMA-456
>                 URL: https://issues.apache.org/jira/browse/HAMA-456
>             Project: Hama
>          Issue Type: Bug
>          Components: bsp
>    Affects Versions: 0.3.0
>            Reporter: Edward J. Yoon
>            Assignee: Edward J. Yoon
>             Fix For: 0.4.0
>
>         Attachments: hama-456_v02.patch, hama-456_v03.patch, patch.txt
>
>
> 1. Add getPeerName(int index) and move mater task election logic into bsp() function.
> 2. getAllPeerNames() will always return null in bsp() function.
> {code}
>   public String[] getAllPeerNames() {
>     String[] result = null;
>     try {
>       result = zk.getChildren("/" + jobConf.getJobID().toString(), this)
>           .toArray(new String[0]);
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira