You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hama.apache.org by "Edward J. Yoon (Created) (JIRA)" <ji...@apache.org> on 2011/10/18 03:02:11 UTC
[jira] [Created] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
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
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
[jira] [Updated] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Edward J. Yoon updated HAMA-456:
--------------------------------
Attachment: patch.txt
This patch adds getPeerName(int index) method which returns the name nth peer from sorted array.
> 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: 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129613#comment-13129613 ]
Edward J. Yoon commented on HAMA-456:
-------------------------------------
Tested on 2 nodes cluster and build also OK.
> 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: 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129632#comment-13129632 ]
Edward J. Yoon commented on HAMA-456:
-------------------------------------
{quote}
Do we need a testcase for that?
{quote}
Basically, yes.
> 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: 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Thomas Jungblut (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129615#comment-13129615 ]
Thomas Jungblut commented on HAMA-456:
--------------------------------------
Looks also good to me.
+1, can be committed.
Do we need a testcase for that?
> 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: 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
[jira] [Updated] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Edward J. Yoon updated HAMA-456:
--------------------------------
Attachment: hama-456_v02.patch
attach v02
> 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, 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129627#comment-13129627 ]
Edward J. Yoon commented on HAMA-456:
-------------------------------------
{code}
/**
* @return the number of peers
*/
public int getPeersNum();
{code}
Let's add also getPeersNum() so that user can get the number of peers easily.
> 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: 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Thomas Jungblut (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129582#comment-13129582 ]
Thomas Jungblut commented on HAMA-456:
--------------------------------------
Additionally I would just sort the array of the children before using the index, so the method returns the same host on other hosts.
If we want to remove getAllPeerNames though, I would throw an UnsupportedOperationException and set this to deprecated with a link to getPeerName(int index).
> 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
>
>
> 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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Thomas Jungblut (Commented) (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 commented on HAMA-456:
--------------------------------------
+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
[jira] [Updated] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Edward J. Yoon updated HAMA-456:
--------------------------------
Attachment: hama-456_v03.patch
Let me commit this patch and then we can start a new issue about refatoring BSPPeerImpl and Barrier Syncronization codes.
> 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
[jira] [Resolved] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Edward J. Yoon (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Edward J. Yoon resolved HAMA-456.
---------------------------------
Resolution: Fixed
Thanks, getPeerName(int index) and getNumPeers() are added.
Looks fine to me. Let's open new ticket.
> 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
[jira] [Issue Comment Edited] (HAMA-456) Add getPeerName(int index)
and Fix getAllPeerNames()
Posted by "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
[jira] [Commented] (HAMA-456) Add getPeerName(int index) and Fix
getAllPeerNames()
Posted by "Thomas Jungblut (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HAMA-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129743#comment-13129743 ]
Thomas Jungblut commented on HAMA-456:
--------------------------------------
I already started the refactoring sync stuff.
I'll open the jira once I have the patch to review.
Thanks.
> 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