You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Mohammad Islam <mi...@yahoo.com> on 2013/10/10 09:51:02 UTC
Review Request 14575: Giraph Application Master: move to new and stable YARN
API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/
-----------------------------------------------------------
Review request for giraph.
Bugs: GIRAPH-737
https://issues.apache.org/jira/browse/GIRAPH-737
Repository: giraph-git
Description
-------
WIP patch. Please give your early comments.
Key points:
* Giraph AM using new API and asynchronous/handler model.
* Adding Kerberos support.
Copied from the JIRA:
Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
Potential impact:
The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
Diffs
-----
giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
pom.xml 41b6bb1
Diff: https://reviews.apache.org/r/14575/diff/
Testing
-------
Thanks,
Mohammad Islam
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Mohammad Islam <mi...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/
-----------------------------------------------------------
(Updated Nov. 1, 2013, 1:48 a.m.)
Review request for giraph.
Changes
-------
Updated with review comments.
Tested with this two commands:
1. mvn clean verify
2. mvn -Phadoop_yarn -Dhadoop.version=2.2.0 clean verify
Bugs: GIRAPH-737
https://issues.apache.org/jira/browse/GIRAPH-737
Repository: giraph-git
Description
-------
WIP patch. Please give your early comments.
Key points:
* Giraph AM using new API and asynchronous/handler model.
* Adding Kerberos support.
Copied from the JIRA:
Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
Potential impact:
The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
Diffs (updated)
-----
README fd9ec20
findbugs-exclude.xml 21aa4ef
giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
pom.xml f2981ff
Diff: https://reviews.apache.org/r/14575/diff/
Testing
-------
Thanks,
Mohammad Islam
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>
> Mohammad Islam wrote:
> Eli thanks for looking into this.
> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
> Please let me know if you want me to check.
>
>
>
> Eli Reisman wrote:
> Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
>
> Eli Reisman wrote:
> Hey Muhammad, wanted to check in with you. I'd love to commit this patch but I'm a but afraid to break the old compatibility (there are people on the mailing list trying Giraph on YARN out still) until I'm sure the test fail is OK. Its the testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats failing for me.
>
> If you are comfortable that you are not getting this error or can help people get things figured out as they trade up to 2.1 YARN API then I'm happy to commit this, it looks great. What do you think?
>
>
> Mohammad Islam wrote:
> Thanks Eli for looking into this.
> I ran the command " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package" for all test cases.
> For your specific failed test case, I tried this : " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package -Dtest=TestFilters -DfailIfNoTests=false"
> Both works fine. Did you try the second command?
>
> But I found out, when I ran all the test cases in my mac, some random test case fail in few instances. But in my Linux desktop (which is more powerful), it succeeds all the time. I think it related to mini cluster and timing.
> Can you please double check the following two items in my patch:
> 1. I changed in two Sasl*.java classes to accommodate the new Exception added in the new API.
> 2. deleted package-info class to make my build succeeds.
>
>
>
>
Thanks Muhammad, I'll try those thing tomorrow night or on the weekend and get the updated patch pushed out. Thanks for this, it looks great!
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Mohammad Islam <mi...@yahoo.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
Eli thanks for looking into this.
The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
Please let me know if you want me to check.
- Mohammad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>
> Mohammad Islam wrote:
> Eli thanks for looking into this.
> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
> Please let me know if you want me to check.
>
>
>
> Eli Reisman wrote:
> Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
Hey Muhammad, wanted to check in with you. I'd love to commit this patch but I'm a but afraid to break the old compatibility (there are people on the mailing list trying Giraph on YARN out still) until I'm sure the test fail is OK. Its the testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats failing for me.
If you are comfortable that you are not getting this error or can help people get things figured out as they trade up to 2.1 YARN API then I'm happy to commit this, it looks great. What do you think?
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>
> Mohammad Islam wrote:
> Eli thanks for looking into this.
> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
> Please let me know if you want me to check.
>
>
Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable YARN API
Posted by Mohammad Islam <mi...@yahoo.com>.
Hi Claudio,
this is definitely the next.
Working on style corrections of the patch. Is there any easy way of complying with Giraph style?
Regards,
Mohammad
On Wednesday, October 30, 2013 10:11 AM, Claudio Martella <cl...@gmail.com> wrote:
Does running Giraph on YARN require particular parameters and configurations. If so, I think as part of this patch (a sub-task), we should provide documentation. What do you think?
On Sat, Oct 26, 2013 at 5:07 PM, Eli Reisman <in...@gmail.com> wrote:
>
>> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
>
>> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>>
>
>> Eli Reisman wrote:
>> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>>
>> Eli Reisman wrote:
>> I'm running this:
>>
>> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>>
>> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>>
>> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>>
>> Mohammad Islam wrote:
>> Eli thanks for looking into this.
>> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
>> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
>> Please let me know if you want me to check.
>>
>>
>>
>> Eli Reisman wrote:
>> Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
>>
>> Eli Reisman wrote:
>> Hey Muhammad, wanted to check in with you. I'd love to commit this patch but I'm a but afraid to break the old compatibility (there are people on the mailing list trying Giraph on YARN out still) until I'm sure the test fail is OK. Its the testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats failing for me.
>>
>> If you are comfortable that you are not getting this error or can help people get things figured out as they trade up to 2.1 YARN API then I'm happy to commit this, it looks great. What do you think?
>>
>>
>> Mohammad Islam wrote:
>> Thanks Eli for looking into this.
>> I ran the command " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package" for all test cases.
>> For your specific failed test case, I tried this : " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package -Dtest=TestFilters -DfailIfNoTests=false"
>> Both works fine. Did you try the second command?
>>
>> But I found out, when I ran all the test cases in my mac, some random test case fail in few instances. But in my Linux desktop (which is more powerful), it succeeds all the time. I think it related to mini cluster and timing.
>> Can you please double check the following two items in my patch:
>> 1. I changed in two Sasl*.java classes to accommodate the new Exception added in the new API.
>> 2. deleted package-info class to make my build succeeds.
>>
>>
>>
>>
>>
>
>> Eli Reisman wrote:
>> Thanks Muhammad, I'll try those thing tomorrow night or on the weekend and get the updated patch pushed out. Thanks for this, it looks great!
>>
>
>Muhammad:
>
>So. Good news: Once i built the project skipping the TestFilter tests once, I _was_ able to get the a second build without skipping the tests to completion! The code is sound and ready to commit, congratulations!
>
>We will need to document on the ticket that a fresh build skipping the test suite before attempting to build against the test suite is required to make the YARN branch build. This is not normal, and we should attempt to correct for it ASAP, but until then I can only commit this if folks have a place to look up the proper procedure to get it to build. Otherwise, I think this work is important enough that we should really get it checked into the codebase fast!
>
>Bad news: upon running the required pre-commit "mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean verify" I got a checkstyle fail on over 342 lines in the new YARN code. These should just be cosmetic fixes, but we need this stuff fixed before I can commit.
>
>Therefore, How about this: I'm going to post your latest patch back on the original JIRA ticket for GIRAPH-737. Please start from there, fix the checkstyle errors, and upload your checkstyle-compliant GIRAPH-737-3.patch also at the JIRA ticket (if you could, its easier for me to pull patches from there.)
>
>Then, as soon as its up, I will commit the patch and we can more forward with more improvements. Great work! Thanks again and sorry to torture you with the checkstyle compliance. Its a pain all of us Giraphers have felt many times!
>
>
>- Eli
>
>
>
>-----------------------------------------------------------
>This is an automatically generated e-mail. To reply, visit:
>https://reviews.apache.org/r/14575/#review26918
>-----------------------------------------------------------
>
>
>
>On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/14575/
>> -----------------------------------------------------------
>>
>> (Updated Oct. 10, 2013, 7:50 a.m.)
>>
>>
>> Review request for giraph.
>>
>>
>> Bugs: GIRAPH-737
>> https://issues.apache.org/jira/browse/GIRAPH-737
>>
>>
>> Repository: giraph-git
>>
>>
>> Description
>> -------
>>
>> WIP patch. Please give your early comments.
>>
>> Key points:
>>
>> * Giraph AM using new API and asynchronous/handler model.
>> * Adding Kerberos support.
>>
>> Copied from the JIRA:
>>
>> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
>> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
>> Potential impact:
>> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>>
>>
>> Diffs
>> -----
>>
>> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
>> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
>> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
>> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
>> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
>> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
>> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
>> pom.xml 41b6bb1
>>
>> Diff: https://reviews.apache.org/r/14575/diff/
>>
>>
>> Testing
>
>> -------
>>
>>
>> Thanks,
>>
>> Mohammad Islam
>>
>>
>
>
--
Claudio Martella
claudio.martella@gmail.com
Re: Review Request 14575: Giraph Application Master: move to new and
stable YARN API
Posted by Claudio Martella <cl...@gmail.com>.
Does running Giraph on YARN require particular parameters and
configurations. If so, I think as part of this patch (a sub-task), we
should provide documentation. What do you think?
On Sat, Oct 26, 2013 at 5:07 PM, Eli Reisman <in...@gmail.com>wrote:
>
>
> > On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the
> beta line now, supporting alpha is no longer such a priority.
> >
> > Eli Reisman wrote:
> > While I'm +1 for moving forward on the upgrade, I'm having a bit of
> trouble getting this patch to build. Let me play with it a bit and post a
> more detailed review in a bit. Thanks Mohammad, looks great!
> >
> > Eli Reisman wrote:
> > I'm running this:
> >
> > mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
> >
> > And getting an NPE on
> org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm
> sure I'm doing something dumb wrong. For one thing, I'm on a Mac running
> Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and
> I'll get this thing reviewed and committed ASAP. Great work!
> >
> > I'll leave a more detailed review once we have this build thing
> squared away. Thanks!
> >
> > Mohammad Islam wrote:
> > Eli thanks for looking into this.
> > The same test passed for me. But I got few non-consistent error. For
> TestYarnJob, I found exception few times. When add a sleep(20000) just
> before the run invocation. It worked fine.
> > I believe all are related to some sort of timing. MiniCluster has
> this type of inconsistency.
> > Please let me know if you want me to check.
> >
> >
> >
> > Eli Reisman wrote:
> > Just to confirm, was I using the right mvn command for your build?
> If you end up reproducing this test fail let me know, maybe its just me. I
> will try to chase down some better info on why the test is failing.
> >
> > Eli Reisman wrote:
> > Hey Muhammad, wanted to check in with you. I'd love to commit this
> patch but I'm a but afraid to break the old compatibility (there are people
> on the mailing list trying Giraph on YARN out still) until I'm sure the
> test fail is OK. Its the
> testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats
> failing for me.
> >
> > If you are comfortable that you are not getting this error or can
> help people get things figured out as they trade up to 2.1 YARN API then
> I'm happy to commit this, it looks great. What do you think?
> >
> >
> > Mohammad Islam wrote:
> > Thanks Eli for looking into this.
> > I ran the command " mvn -Phadoop_yarn
> -Dhadoop.version=2.1.1-SNAPSHOT clean package" for all test cases.
> > For your specific failed test case, I tried this : " mvn
> -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package
> -Dtest=TestFilters -DfailIfNoTests=false"
> > Both works fine. Did you try the second command?
> >
> > But I found out, when I ran all the test cases in my mac, some
> random test case fail in few instances. But in my Linux desktop (which is
> more powerful), it succeeds all the time. I think it related to mini
> cluster and timing.
> > Can you please double check the following two items in my patch:
> > 1. I changed in two Sasl*.java classes to accommodate the new
> Exception added in the new API.
> > 2. deleted package-info class to make my build succeeds.
> >
> >
> >
> >
> >
> > Eli Reisman wrote:
> > Thanks Muhammad, I'll try those thing tomorrow night or on the
> weekend and get the updated patch pushed out. Thanks for this, it looks
> great!
> >
>
> Muhammad:
>
> So. Good news: Once i built the project skipping the TestFilter tests
> once, I _was_ able to get the a second build without skipping the tests to
> completion! The code is sound and ready to commit, congratulations!
>
> We will need to document on the ticket that a fresh build skipping the
> test suite before attempting to build against the test suite is required to
> make the YARN branch build. This is not normal, and we should attempt to
> correct for it ASAP, but until then I can only commit this if folks have a
> place to look up the proper procedure to get it to build. Otherwise, I
> think this work is important enough that we should really get it checked
> into the codebase fast!
>
> Bad news: upon running the required pre-commit "mvn -Phadoop_yarn
> -Dhadoop.version=2.1.1-SNAPSHOT clean verify" I got a checkstyle fail on
> over 342 lines in the new YARN code. These should just be cosmetic fixes,
> but we need this stuff fixed before I can commit.
>
> Therefore, How about this: I'm going to post your latest patch back on the
> original JIRA ticket for GIRAPH-737. Please start from there, fix the
> checkstyle errors, and upload your checkstyle-compliant GIRAPH-737-3.patch
> also at the JIRA ticket (if you could, its easier for me to pull patches
> from there.)
>
> Then, as soon as its up, I will commit the patch and we can more forward
> with more improvements. Great work! Thanks again and sorry to torture you
> with the checkstyle compliance. Its a pain all of us Giraphers have felt
> many times!
>
>
> - Eli
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/#review26918
> -----------------------------------------------------------
>
>
> On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/14575/
> > -----------------------------------------------------------
> >
> > (Updated Oct. 10, 2013, 7:50 a.m.)
> >
> >
> > Review request for giraph.
> >
> >
> > Bugs: GIRAPH-737
> > https://issues.apache.org/jira/browse/GIRAPH-737
> >
> >
> > Repository: giraph-git
> >
> >
> > Description
> > -------
> >
> > WIP patch. Please give your early comments.
> >
> > Key points:
> >
> > * Giraph AM using new API and asynchronous/handler model.
> > * Adding Kerberos support.
> >
> > Copied from the JIRA:
> >
> > Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a
> Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn
> significantly overhauled its APIs and associated coding patterns. The new
> beta version is 2.1.x and I was told by Yarn-dev that current APIs will not
> change much.
> > In the above circumstances, we need to substantially overhaul Giraph AM
> as well to accommodate with the new Yarn API. Moreover, in newer YARN API,
> supporting kerberos security in AM becomes easier and more transparent.
> > Potential impact:
> > The upcoming Girpah AM will not work with earlier alpha Hadoop versions
> such as 2.0.3. I'm not sure if anyone is using Giraph AM in production.
> However, the more prevalent way of Giraph processing (MR-based) should
> continue to work.
> >
> >
> > Diffs
> > -----
> >
> >
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java
> 00a802f
> >
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java
> 922f373
> >
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java
> c2b88a0
> > giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java
> 341db0e
> > giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> >
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java
> 65d87e3
> >
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java
> c2327ca
> > pom.xml 41b6bb1
> >
> > Diff: https://reviews.apache.org/r/14575/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Mohammad Islam
> >
> >
>
>
--
Claudio Martella
claudio.martella@gmail.com
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>
> Mohammad Islam wrote:
> Eli thanks for looking into this.
> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
> Please let me know if you want me to check.
>
>
>
> Eli Reisman wrote:
> Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
>
> Eli Reisman wrote:
> Hey Muhammad, wanted to check in with you. I'd love to commit this patch but I'm a but afraid to break the old compatibility (there are people on the mailing list trying Giraph on YARN out still) until I'm sure the test fail is OK. Its the testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats failing for me.
>
> If you are comfortable that you are not getting this error or can help people get things figured out as they trade up to 2.1 YARN API then I'm happy to commit this, it looks great. What do you think?
>
>
> Mohammad Islam wrote:
> Thanks Eli for looking into this.
> I ran the command " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package" for all test cases.
> For your specific failed test case, I tried this : " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package -Dtest=TestFilters -DfailIfNoTests=false"
> Both works fine. Did you try the second command?
>
> But I found out, when I ran all the test cases in my mac, some random test case fail in few instances. But in my Linux desktop (which is more powerful), it succeeds all the time. I think it related to mini cluster and timing.
> Can you please double check the following two items in my patch:
> 1. I changed in two Sasl*.java classes to accommodate the new Exception added in the new API.
> 2. deleted package-info class to make my build succeeds.
>
>
>
>
>
> Eli Reisman wrote:
> Thanks Muhammad, I'll try those thing tomorrow night or on the weekend and get the updated patch pushed out. Thanks for this, it looks great!
>
Muhammad:
So. Good news: Once i built the project skipping the TestFilter tests once, I _was_ able to get the a second build without skipping the tests to completion! The code is sound and ready to commit, congratulations!
We will need to document on the ticket that a fresh build skipping the test suite before attempting to build against the test suite is required to make the YARN branch build. This is not normal, and we should attempt to correct for it ASAP, but until then I can only commit this if folks have a place to look up the proper procedure to get it to build. Otherwise, I think this work is important enough that we should really get it checked into the codebase fast!
Bad news: upon running the required pre-commit "mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean verify" I got a checkstyle fail on over 342 lines in the new YARN code. These should just be cosmetic fixes, but we need this stuff fixed before I can commit.
Therefore, How about this: I'm going to post your latest patch back on the original JIRA ticket for GIRAPH-737. Please start from there, fix the checkstyle errors, and upload your checkstyle-compliant GIRAPH-737-3.patch also at the JIRA ticket (if you could, its easier for me to pull patches from there.)
Then, as soon as its up, I will commit the patch and we can more forward with more improvements. Great work! Thanks again and sorry to torture you with the checkstyle compliance. Its a pain all of us Giraphers have felt many times!
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and
stable YARN API
Posted by Claudio Martella <cl...@gmail.com>.
Yes, Armando also experienced failing tests on clean code that would pass
on my faster machine.
On Wed, Oct 23, 2013 at 3:49 AM, Mohammad Islam <mi...@yahoo.com> wrote:
>
>
> > On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the
> beta line now, supporting alpha is no longer such a priority.
> >
> > Eli Reisman wrote:
> > While I'm +1 for moving forward on the upgrade, I'm having a bit of
> trouble getting this patch to build. Let me play with it a bit and post a
> more detailed review in a bit. Thanks Mohammad, looks great!
> >
> > Eli Reisman wrote:
> > I'm running this:
> >
> > mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
> >
> > And getting an NPE on
> org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm
> sure I'm doing something dumb wrong. For one thing, I'm on a Mac running
> Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and
> I'll get this thing reviewed and committed ASAP. Great work!
> >
> > I'll leave a more detailed review once we have this build thing
> squared away. Thanks!
> >
> > Mohammad Islam wrote:
> > Eli thanks for looking into this.
> > The same test passed for me. But I got few non-consistent error. For
> TestYarnJob, I found exception few times. When add a sleep(20000) just
> before the run invocation. It worked fine.
> > I believe all are related to some sort of timing. MiniCluster has
> this type of inconsistency.
> > Please let me know if you want me to check.
> >
> >
> >
> > Eli Reisman wrote:
> > Just to confirm, was I using the right mvn command for your build?
> If you end up reproducing this test fail let me know, maybe its just me. I
> will try to chase down some better info on why the test is failing.
> >
> > Eli Reisman wrote:
> > Hey Muhammad, wanted to check in with you. I'd love to commit this
> patch but I'm a but afraid to break the old compatibility (there are people
> on the mailing list trying Giraph on YARN out still) until I'm sure the
> test fail is OK. Its the
> testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats
> failing for me.
> >
> > If you are comfortable that you are not getting this error or can
> help people get things figured out as they trade up to 2.1 YARN API then
> I'm happy to commit this, it looks great. What do you think?
> >
>
> Thanks Eli for looking into this.
> I ran the command " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT
> clean package" for all test cases.
> For your specific failed test case, I tried this : " mvn -Phadoop_yarn
> -Dhadoop.version=2.1.1-SNAPSHOT clean package -Dtest=TestFilters
> -DfailIfNoTests=false"
> Both works fine. Did you try the second command?
>
> But I found out, when I ran all the test cases in my mac, some random test
> case fail in few instances. But in my Linux desktop (which is more
> powerful), it succeeds all the time. I think it related to mini cluster and
> timing.
> Can you please double check the following two items in my patch:
> 1. I changed in two Sasl*.java classes to accommodate the new Exception
> added in the new API.
> 2. deleted package-info class to make my build succeeds.
>
>
>
> - Mohammad
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/#review26918
> -----------------------------------------------------------
>
>
> On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/14575/
> > -----------------------------------------------------------
> >
> > (Updated Oct. 10, 2013, 7:50 a.m.)
> >
> >
> > Review request for giraph.
> >
> >
> > Bugs: GIRAPH-737
> > https://issues.apache.org/jira/browse/GIRAPH-737
> >
> >
> > Repository: giraph-git
> >
> >
> > Description
> > -------
> >
> > WIP patch. Please give your early comments.
> >
> > Key points:
> >
> > * Giraph AM using new API and asynchronous/handler model.
> > * Adding Kerberos support.
> >
> > Copied from the JIRA:
> >
> > Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a
> Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn
> significantly overhauled its APIs and associated coding patterns. The new
> beta version is 2.1.x and I was told by Yarn-dev that current APIs will not
> change much.
> > In the above circumstances, we need to substantially overhaul Giraph AM
> as well to accommodate with the new Yarn API. Moreover, in newer YARN API,
> supporting kerberos security in AM becomes easier and more transparent.
> > Potential impact:
> > The upcoming Girpah AM will not work with earlier alpha Hadoop versions
> such as 2.0.3. I'm not sure if anyone is using Giraph AM in production.
> However, the more prevalent way of Giraph processing (MR-based) should
> continue to work.
> >
> >
> > Diffs
> > -----
> >
> >
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java
> 00a802f
> >
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java
> 922f373
> >
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java
> c2b88a0
> > giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java
> 341db0e
> > giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> >
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java
> 65d87e3
> >
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java
> c2327ca
> > pom.xml 41b6bb1
> >
> > Diff: https://reviews.apache.org/r/14575/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Mohammad Islam
> >
> >
>
>
--
Claudio Martella
claudio.martella@gmail.com
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Mohammad Islam <mi...@yahoo.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
>
> Eli Reisman wrote:
> I'm running this:
>
> mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
>
> And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
>
> I'll leave a more detailed review once we have this build thing squared away. Thanks!
>
> Mohammad Islam wrote:
> Eli thanks for looking into this.
> The same test passed for me. But I got few non-consistent error. For TestYarnJob, I found exception few times. When add a sleep(20000) just before the run invocation. It worked fine.
> I believe all are related to some sort of timing. MiniCluster has this type of inconsistency.
> Please let me know if you want me to check.
>
>
>
> Eli Reisman wrote:
> Just to confirm, was I using the right mvn command for your build? If you end up reproducing this test fail let me know, maybe its just me. I will try to chase down some better info on why the test is failing.
>
> Eli Reisman wrote:
> Hey Muhammad, wanted to check in with you. I'd love to commit this patch but I'm a but afraid to break the old compatibility (there are people on the mailing list trying Giraph on YARN out still) until I'm sure the test fail is OK. Its the testVertexFilter(org.apache.giraph.io.TestFilters) Edge filter test thats failing for me.
>
> If you are comfortable that you are not getting this error or can help people get things figured out as they trade up to 2.1 YARN API then I'm happy to commit this, it looks great. What do you think?
>
Thanks Eli for looking into this.
I ran the command " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package" for all test cases.
For your specific failed test case, I tried this : " mvn -Phadoop_yarn -Dhadoop.version=2.1.1-SNAPSHOT clean package -Dtest=TestFilters -DfailIfNoTests=false"
Both works fine. Did you try the second command?
But I found out, when I ran all the test cases in my mac, some random test case fail in few instances. But in my Linux desktop (which is more powerful), it succeeds all the time. I think it related to mini cluster and timing.
Can you please double check the following two items in my patch:
1. I changed in two Sasl*.java classes to accommodate the new Exception added in the new API.
2. deleted package-info class to make my build succeeds.
- Mohammad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
> On Oct. 11, 2013, 1:52 a.m., Eli Reisman wrote:
> > I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
>
> Eli Reisman wrote:
> While I'm +1 for moving forward on the upgrade, I'm having a bit of trouble getting this patch to build. Let me play with it a bit and post a more detailed review in a bit. Thanks Mohammad, looks great!
I'm running this:
mvn -Dhadoop.version=2.1.1-SNAPSHOT -Phadoop_yarn install
And getting an NPE on org.apache.giraph.io.TestFilters#testEdgeFilter that fails the test. I'm sure I'm doing something dumb wrong. For one thing, I'm on a Mac running Java 1.6_40? Anyway if I'm doing something wrong Mohammad let me know and I'll get this thing reviewed and committed ASAP. Great work!
I'll leave a more detailed review once we have this build thing squared away. Thanks!
- Eli
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>
Re: Review Request 14575: Giraph Application Master: move to new and stable
YARN API
Posted by Eli Reisman <in...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14575/#review26918
-----------------------------------------------------------
Ship it!
I'm +1 for moving to the new YARN APIs. I think with Hadoop 2 on the beta line now, supporting alpha is no longer such a priority.
- Eli Reisman
On Oct. 10, 2013, 7:50 a.m., Mohammad Islam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14575/
> -----------------------------------------------------------
>
> (Updated Oct. 10, 2013, 7:50 a.m.)
>
>
> Review request for giraph.
>
>
> Bugs: GIRAPH-737
> https://issues.apache.org/jira/browse/GIRAPH-737
>
>
> Repository: giraph-git
>
>
> Description
> -------
>
> WIP patch. Please give your early comments.
>
> Key points:
>
> * Giraph AM using new API and asynchronous/handler model.
> * Adding Kerberos support.
>
> Copied from the JIRA:
>
> Giraph was the early adopter of Hadoop YARN AM! Eli successfully wrote a Giraph AM based on Hadoop 2.0.x_alpha. However, in last few months, Yarn significantly overhauled its APIs and associated coding patterns. The new beta version is 2.1.x and I was told by Yarn-dev that current APIs will not change much.
> In the above circumstances, we need to substantially overhaul Giraph AM as well to accommodate with the new Yarn API. Moreover, in newer YARN API, supporting kerberos security in AM becomes easier and more transparent.
> Potential impact:
> The upcoming Girpah AM will not work with earlier alpha Hadoop versions such as 2.0.3. I'm not sure if anyone is using Giraph AM in production. However, the more prevalent way of Giraph processing (MR-based) should continue to work.
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/comm/netty/SaslNettyServer.java 00a802f
> giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/SaslServerHandler.java 922f373
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphApplicationMaster.java c2b88a0
> giraph-core/src/main/java/org/apache/giraph/yarn/GiraphYarnClient.java 341db0e
> giraph-core/src/main/java/org/apache/giraph/yarn/YarnUtils.java aa042e8
> giraph-hive/src/main/java/org/apache/giraph/hive/output/package-info.java 65d87e3
> giraph-hive/src/test/java/org/apache/giraph/hive/input/package-info.java c2327ca
> pom.xml 41b6bb1
>
> Diff: https://reviews.apache.org/r/14575/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mohammad Islam
>
>