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