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 <[email protected]>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 [email protected]
