Giraph
  1. Giraph
  2. GIRAPH-153

HBase/Accumulo Input and Output formats

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.1.0
    • Fix Version/s: None
    • Component/s: bsp
    • Labels:
      None
    • Environment:

      Single host OSX 10.6.8 2.2Ghz Intel i7, 8GB

      Description

      Four abstract classes that wrap their respective delegate input/output formats for
      easy hooks into vertex input format subclasses. I've included some sample programs that show two very simple graph
      algorithms. I have a graph generator that builds out a very simple directed structure, starting with a few 'root' nodes.

      Root nodes are defined as nodes which are not listed as a child anywhere in the graph.

      Algorithm 1) AccumuloRootMarker.java --> Accumulo as read/write source. Every vertex starts thinking it's a root. At superstep 0, send a message down to each
      child as a non-root notification. After superstep 1, only root nodes will have never been messaged.

      Algorithm 2) TableRootMarker --> HBase as read/write source. Expands on A1 by bundling the notification logic followed by root node propagation. Once we've marked the appropriate nodes as roots, tell every child which roots it can be traced back to via one or more spanning trees. This will take N + 2 supersteps where N is the maximum number of hops from any root to any leaf, plus 2 supersteps for the initial root flagging.

      I've included all relevant code plus DistributedCacheHelper.java for recursive cache file and archive searches. It is more hadoop centric than giraph, but these jobs use it so I figured why not commit here.

      These have been tested through local JobRunner, pseudo-distributed on the aforementioned hardware, and full distributed on EC2. More details in the comments.

      1. GIRAPH-153.3.patch
        77 kB
        Brian Femiano
      2. GIRAPH-153.2.patch
        244 kB
        Brian Femiano
      3. GIRAPH-153.1.patch
        85 kB
        Brian Femiano
      4. GIRAPH-153.patch
        80 kB
        Brian Femiano

        Issue Links

          Activity

          Hide
          Brian Femiano added a comment -

          More on the algorithms. This was using the LineRecordReader from flat text files in HDFS. Reading from HBase adds about 20min of setup overhead.
          My graph nodes take the form of

          0000001 -1 -1 .... 000006, 0000007
          where 0000001 = node guid
          and 000006, and 000007 are children linked to that guid. A child can belong to more than 1 guid, hence the formation of a graph instead of a simple tree.

          I have tested these against 10 and 19 node clusters using m1.4xlarge instances via EC2. 70GB, 8 virtual cores per machine. GigE backplane bandwidth.

          Datasets included 98mil with 1050 possible hops from a given root to any leaf.

          The 98mil runs in about 1 hour using 143 workers (144 map tasks -1 for the master). Adding more workers with more concurrent map slots does not lead to better performance. In fact running with over 150 workers will cause FS open descriptor limit issues, even when increasing the ulimit for all users on each machine and bouncing the cluster. The algorithm and dataset exhibit the best performance on as few a workers as possible necessary to instantiate all 98mil nodes. 10 seems to be the sweet spot. There's just enough heap per JVM for all 79 workers to load the graph in a responsible amount of time, without incurring the stack overhead of having too many workers to open a communication channel to. I understand there is 'num_flush_thread' and other configuration parameters designed to control this, but I haven't experimented with them yet. This is over Federa 8 AMI images with 7GB heap per worker JVM.

          Running just a small sample (1mil nodes) over 160+ workers leads to similar results. "Unable to create new native thread" or "Too many open FS".
          Throttle back the number of workers causes the symptoms to go away. I've been following Giraph-45 which I believe is related.

          I'm happy to expand on these issues for anyone interested.

          I also worked on a variant algorithm that passes each node in the graph a copy of every spanning tree from each root of which the node is connected back to. It does not adhere to strict connected component rules, just for reference. Naturally this lead to a much higher volume of message traffic and gc limit overhead reached issues. The 98mil graph would produce around 11billion messages in the first 5 supersteps. Concatenating the spanning trees together as one message to limit the overall # of messages would lead to OOM issues. I don't have a requirement to perform this nasty a BSP algorithm, but it was an interesting stress test.

          Show
          Brian Femiano added a comment - More on the algorithms. This was using the LineRecordReader from flat text files in HDFS. Reading from HBase adds about 20min of setup overhead. My graph nodes take the form of 0000001 -1 -1 .... 000006, 0000007 where 0000001 = node guid and 000006, and 000007 are children linked to that guid. A child can belong to more than 1 guid, hence the formation of a graph instead of a simple tree. I have tested these against 10 and 19 node clusters using m1.4xlarge instances via EC2. 70GB, 8 virtual cores per machine. GigE backplane bandwidth. Datasets included 98mil with 1050 possible hops from a given root to any leaf. The 98mil runs in about 1 hour using 143 workers (144 map tasks -1 for the master). Adding more workers with more concurrent map slots does not lead to better performance. In fact running with over 150 workers will cause FS open descriptor limit issues, even when increasing the ulimit for all users on each machine and bouncing the cluster. The algorithm and dataset exhibit the best performance on as few a workers as possible necessary to instantiate all 98mil nodes. 10 seems to be the sweet spot. There's just enough heap per JVM for all 79 workers to load the graph in a responsible amount of time, without incurring the stack overhead of having too many workers to open a communication channel to. I understand there is 'num_flush_thread' and other configuration parameters designed to control this, but I haven't experimented with them yet. This is over Federa 8 AMI images with 7GB heap per worker JVM. Running just a small sample (1mil nodes) over 160+ workers leads to similar results. "Unable to create new native thread" or "Too many open FS". Throttle back the number of workers causes the symptoms to go away. I've been following Giraph-45 which I believe is related. I'm happy to expand on these issues for anyone interested. I also worked on a variant algorithm that passes each node in the graph a copy of every spanning tree from each root of which the node is connected back to. It does not adhere to strict connected component rules, just for reference. Naturally this lead to a much higher volume of message traffic and gc limit overhead reached issues. The 98mil graph would produce around 11billion messages in the first 5 supersteps. Concatenating the spanning trees together as one message to limit the overall # of messages would lead to OOM issues. I don't have a requirement to perform this nasty a BSP algorithm, but it was an interesting stress test.
          Hide
          Eugene Koontz added a comment -

          Brian mentions that GIRAPH-45 might be related to the file descriptor limit he experienced while testing with a high number of workers.

          Show
          Eugene Koontz added a comment - Brian mentions that GIRAPH-45 might be related to the file descriptor limit he experienced while testing with a high number of workers.
          Hide
          Avery Ching added a comment -

          Brian, this is an awesome contribution and a lot of code. I'm really sorry that it took me so long to look at this. Is there any change that you could add some simple unittests for your formats? TestJsonBase64Format.java is an example that might be easy to adapt for your formats.

          Also, I just created a page for how to contribute. https://cwiki.apache.org/confluence/display/GIRAPH/How+to+Contribute

          Have you run 'mvn verify'? Thanks!

          Show
          Avery Ching added a comment - Brian, this is an awesome contribution and a lot of code. I'm really sorry that it took me so long to look at this. Is there any change that you could add some simple unittests for your formats? TestJsonBase64Format.java is an example that might be easy to adapt for your formats. Also, I just created a page for how to contribute. https://cwiki.apache.org/confluence/display/GIRAPH/How+to+Contribute Have you run 'mvn verify'? Thanks!
          Hide
          Brian Femiano added a comment -

          No worries Avery. Thanks for looking at it.

          I'll read the wiki page, add some unit tests and verify everything.

          Show
          Brian Femiano added a comment - No worries Avery. Thanks for looking at it. I'll read the wiki page, add some unit tests and verify everything.
          Hide
          Avery Ching added a comment -

          Brian, could you make it a single patch for us to take a look at? I'm excited to see this work.

          Show
          Avery Ching added a comment - Brian, could you make it a single patch for us to take a look at? I'm excited to see this work.
          Hide
          Brian Femiano added a comment -

          For HBase I'm still working on the unit tests. I hope to have something by end of Sunday.

          Accumulo has no published maven artifacts, and since I'm fairly certain we don't want a provided jar, that
          will have to wait. I'll post something on the dev group and see what responses I get.

          Show
          Brian Femiano added a comment - For HBase I'm still working on the unit tests. I hope to have something by end of Sunday. Accumulo has no published maven artifacts, and since I'm fairly certain we don't want a provided jar, that will have to wait. I'll post something on the dev group and see what responses I get.
          Hide
          Brian Femiano added a comment -

          The Accumulo team is about to release a new version that should have a published maven
          artifact.

          I'm concerned with how fat the jar becomes once the HBase core files are coalesced into the Giraph jar.

          It goes from a reasonable 5MB in size to 34MB. This causes quite a slow down with the distributed
          unit tests. We may want to consider having the Hbase-contrib in a separate submodule, much the same
          way Hive does with the HBaseStorageHandler. Giraph users that desire HBase support will need the main
          giraph jar, the hbase-contrib-jar, and any hbase dependencies.

          Thoughts?

          Show
          Brian Femiano added a comment - The Accumulo team is about to release a new version that should have a published maven artifact. I'm concerned with how fat the jar becomes once the HBase core files are coalesced into the Giraph jar. It goes from a reasonable 5MB in size to 34MB. This causes quite a slow down with the distributed unit tests. We may want to consider having the Hbase-contrib in a separate submodule, much the same way Hive does with the HBaseStorageHandler. Giraph users that desire HBase support will need the main giraph jar, the hbase-contrib-jar, and any hbase dependencies. Thoughts?
          Hide
          Avery Ching added a comment -

          34 MB is huge. Can we do something like make the dependency scope provided and then use the distributed cache for unittests?

          Show
          Avery Ching added a comment - 34 MB is huge. Can we do something like make the dependency scope provided and then use the distributed cache for unittests?
          Hide
          Jakob Homan added a comment -

          I'm concerned with how fat the jar becomes once the HBase core files are coalesced into the Giraph jar.

          This is a great effort, but will have to be done in some other way than just including a direct dependency on hbase into Giraph. Lots of sites already have a different HBase installed and this will just cause headaches for them. Alternatively, for those sites that don't use HBase (and may not want it on their clusters) these jars as part of Giraph isn't a viable option. Basically, making Giraph depend on HBase is a non-starter.

          Can maven modules help us out here? Can we have a separate artifact, giraph-hbase-formats.jar or something, we can publish that those that wish this functionality can pull in? That jar can depend on both hbase and giraph with no extra requirement on either of those projects.

          Show
          Jakob Homan added a comment - I'm concerned with how fat the jar becomes once the HBase core files are coalesced into the Giraph jar. This is a great effort, but will have to be done in some other way than just including a direct dependency on hbase into Giraph. Lots of sites already have a different HBase installed and this will just cause headaches for them. Alternatively, for those sites that don't use HBase (and may not want it on their clusters) these jars as part of Giraph isn't a viable option. Basically, making Giraph depend on HBase is a non-starter. Can maven modules help us out here? Can we have a separate artifact, giraph-hbase-formats.jar or something, we can publish that those that wish this functionality can pull in? That jar can depend on both hbase and giraph with no extra requirement on either of those projects.
          Hide
          Brian Femiano added a comment -

          Jakob, that's exactly along the lines of what I was thinking. A separate module that builds along side the main giraph jar for extra
          functionality. Users can see which version of HBase we've compiled against.

          People can use this 'giraph-hbase-contrib.jar' by including giraph, a compatible version of HBase, and all related dependencies on the classpath.

          To build, it will list giraph as a dependency in maven.

          Let me finish up my unit tests this week and I'll post a patch along with the new files.

          The equivalent Accumulo support will take a little longer pending their published maven artifact.

          Show
          Brian Femiano added a comment - Jakob, that's exactly along the lines of what I was thinking. A separate module that builds along side the main giraph jar for extra functionality. Users can see which version of HBase we've compiled against. People can use this 'giraph-hbase-contrib.jar' by including giraph, a compatible version of HBase, and all related dependencies on the classpath. To build, it will list giraph as a dependency in maven. Let me finish up my unit tests this week and I'll post a patch along with the new files. The equivalent Accumulo support will take a little longer pending their published maven artifact.
          Hide
          Brian Femiano added a comment -

          Avery and Jakob. Here's what I've got setup. I wanted to double-check this before moving
          forward with the project template.

          1) I have a subproject 'giraph-formats-contrib' under the giraph trunk that depends on giraph 0.2-SNAPSHOT. Since this is not yet hosted in maven central I installed it to my local repo. Note this is only necessary if you wish to build the subproject. Not this is not a maven submodule that builds as a dependency. It's entirely standalone.

          2) The subproject hosts the Accumulo 1.4.0 and HBase 0.92.1 abstract input/output formats, and any future derived implementations.

          3) I copied the BspCase Junit class into the subproject redundantly. The subproject is builds and tests entirly standalone from the main giraph build, except for the dependency giraph.jar. Unfortuantely, the test classes are not included in the fat jar, so I copied one class into the build for future unit testing.

          I'm moving forward with the unit tests. If you guys have think I should change anything I'll happily rework my structure. The main thing I strived for was total separation from the main build. It simply uses Giraph as a jar dependency.

          Show
          Brian Femiano added a comment - Avery and Jakob. Here's what I've got setup. I wanted to double-check this before moving forward with the project template. 1) I have a subproject 'giraph-formats-contrib' under the giraph trunk that depends on giraph 0.2-SNAPSHOT. Since this is not yet hosted in maven central I installed it to my local repo. Note this is only necessary if you wish to build the subproject. Not this is not a maven submodule that builds as a dependency. It's entirely standalone. 2) The subproject hosts the Accumulo 1.4.0 and HBase 0.92.1 abstract input/output formats, and any future derived implementations. 3) I copied the BspCase Junit class into the subproject redundantly. The subproject is builds and tests entirly standalone from the main giraph build, except for the dependency giraph.jar. Unfortuantely, the test classes are not included in the fat jar, so I copied one class into the build for future unit testing. I'm moving forward with the unit tests. If you guys have think I should change anything I'll happily rework my structure. The main thing I strived for was total separation from the main build. It simply uses Giraph as a jar dependency.
          Hide
          Jakob Homan added a comment -

          I have a subproject 'giraph-formats-contrib'

          This sounds like a good name as we can also stash the Hive work Avery has done there.

          Not this is not a maven submodule that builds as a dependency. It's entirely standalone.

          What are the advantages of this approach compard to a maven submodule (keeping in mind that I'm a Maven moron)?

          Show
          Jakob Homan added a comment - I have a subproject 'giraph-formats-contrib' This sounds like a good name as we can also stash the Hive work Avery has done there. Not this is not a maven submodule that builds as a dependency. It's entirely standalone. What are the advantages of this approach compard to a maven submodule (keeping in mind that I'm a Maven moron)?
          Hide
          Brian Femiano added a comment -

          Maven submodules introduce build dependencies into the main parent build. The giraph-formats-contrib submodule would have
          to be built first even if the user had no need for it, which is not what we want. Submodules allow individual components to be built
          independently and then grouped together in one final build. You can build the submodule in isolation and inherit the dependencies
          from the parent pom.

          An advantage is you can isolate specific dependencies into groups based on what submodules need them. The main disadvantage is that it would chain the giraph-formats-contrib as part of the main giraph.jar build, when it is infact not a dependency.

          To counter this I've built a subproject, managed by maven, that lives as a subdirectory within the main giraph trunk. It lists giraph as a jar dependency and builds standalone. It does not introduce any parent->child build relationships for the contrib module. Anyone who simply wishes to build the main giraph.jar will not see it. It does however require giraph.jar to be installed in the users local maven repo, at least until giraph is hosted in maven central or some other nexus.

          Hope that explains it somewhat. If you guys would rather submodules or see a glaring issue with this approach, I'm happy to readjust.

          Show
          Brian Femiano added a comment - Maven submodules introduce build dependencies into the main parent build. The giraph-formats-contrib submodule would have to be built first even if the user had no need for it, which is not what we want. Submodules allow individual components to be built independently and then grouped together in one final build. You can build the submodule in isolation and inherit the dependencies from the parent pom. An advantage is you can isolate specific dependencies into groups based on what submodules need them. The main disadvantage is that it would chain the giraph-formats-contrib as part of the main giraph.jar build, when it is infact not a dependency. To counter this I've built a subproject, managed by maven, that lives as a subdirectory within the main giraph trunk. It lists giraph as a jar dependency and builds standalone. It does not introduce any parent->child build relationships for the contrib module. Anyone who simply wishes to build the main giraph.jar will not see it. It does however require giraph.jar to be installed in the users local maven repo, at least until giraph is hosted in maven central or some other nexus. Hope that explains it somewhat. If you guys would rather submodules or see a glaring issue with this approach, I'm happy to readjust.
          Hide
          Avery Ching added a comment -

          From what you've described, sounds good to me. In the worst case, we can change it to a submodule if that makes more sense in the future. I would like to use a similar approach for https://issues.apache.org/jira/browse/GIRAPH-93, as Jakob mentioned.

          Show
          Avery Ching added a comment - From what you've described, sounds good to me. In the worst case, we can change it to a submodule if that makes more sense in the future. I would like to use a similar approach for https://issues.apache.org/jira/browse/GIRAPH-93 , as Jakob mentioned.
          Hide
          Jakob Homan added a comment -

          Sounds good to me as well. I'm fine with devs having to build/test against this subproject/module; this ensures we don't get out of synch with our adapters. My mail goal is to make sure anyone wanting just Giraph doesn't need the hbase/accumulo stuff and it sounds like this does that. Thanks for the hard work, Brian.

          Show
          Jakob Homan added a comment - Sounds good to me as well. I'm fine with devs having to build/test against this subproject/module; this ensures we don't get out of synch with our adapters. My mail goal is to make sure anyone wanting just Giraph doesn't need the hbase/accumulo stuff and it sounds like this does that. Thanks for the hard work, Brian.
          Hide
          Brian Femiano added a comment -

          Patch contains the entire submodule including HBase and Accumulo unit tests. It has been tested against Accumulo 1.4 (latest release) and HBase 0.90.5 with Zookeeper 3.3.3. It includes 4 abstract classes designed to help subclass reading/writing to and from these datastores.

          The test package shows a few example subclasses which were needed to verify the behavior. For now they only run in local mode and will be disabled if the user supplies a jobtracker URI.

          It builds exactly as described in the earlier comments. Simply run 'mvn verify' and you'll get an isolated build.

          A few caveats:

          1) Users must 'mvn install' the giraph artifact in their local repo, at least until we get something posted on maven central.
          2) I modified the pom.xml to exclude the artifact from the rat plugin. I realize this is less than desirable, but I couldn't get anything running
          despite numerous attempts at fixing the "too many unapproved licenses" issues. I'm interested to hear your guys thoughts.
          3) Duplicate BspCase in my submodule, at least until Giraph has a test artifact. '
          4) Initializing the AccumuloVertexInputFormat has some procedural limitations inherent in the format design when run with the GiraphJob. It really expects to have control of the Job instance. These can be difficult to track down. I tried to document these in my unit tests and provide some simple error wrapping to help notify users when they see these.
          5) No README.txt or any wiki entry yet. I figured I'd wait and see what feedback you guys had.

          Hopefully people will find the submodule useful.

          Show
          Brian Femiano added a comment - Patch contains the entire submodule including HBase and Accumulo unit tests. It has been tested against Accumulo 1.4 (latest release) and HBase 0.90.5 with Zookeeper 3.3.3. It includes 4 abstract classes designed to help subclass reading/writing to and from these datastores. The test package shows a few example subclasses which were needed to verify the behavior. For now they only run in local mode and will be disabled if the user supplies a jobtracker URI. It builds exactly as described in the earlier comments. Simply run 'mvn verify' and you'll get an isolated build. A few caveats: 1) Users must 'mvn install' the giraph artifact in their local repo, at least until we get something posted on maven central. 2) I modified the pom.xml to exclude the artifact from the rat plugin. I realize this is less than desirable, but I couldn't get anything running despite numerous attempts at fixing the "too many unapproved licenses" issues. I'm interested to hear your guys thoughts. 3) Duplicate BspCase in my submodule, at least until Giraph has a test artifact. ' 4) Initializing the AccumuloVertexInputFormat has some procedural limitations inherent in the format design when run with the GiraphJob. It really expects to have control of the Job instance. These can be difficult to track down. I tried to document these in my unit tests and provide some simple error wrapping to help notify users when they see these. 5) No README.txt or any wiki entry yet. I figured I'd wait and see what feedback you guys had. Hopefully people will find the submodule useful.
          Hide
          Brian Femiano added a comment -

          Per Keith Turner's comments in HAMA-153 would it make more sense to host this submodule on github?

          Show
          Brian Femiano added a comment - Per Keith Turner's comments in HAMA-153 would it make more sense to host this submodule on github?
          Hide
          Brian Femiano added a comment -

          Sorry HAMA-544

          Show
          Brian Femiano added a comment - Sorry HAMA-544
          Hide
          Avery Ching added a comment -

          I think hosting the submodule on github would produce one more barrier to entry. I prefer to have it with Giraph directly. Anyone else?

          Show
          Avery Ching added a comment - I think hosting the submodule on github would produce one more barrier to entry. I prefer to have it with Giraph directly. Anyone else?
          Hide
          Jakob Homan added a comment -

          Per Keith Turner's comments in HAMA-153 would it make more sense to host this submodule on github?

          I've spent lots of time doing this with the Avro connector for Hive and wish I hadn't. It's quite easy for the connector code to drift from the main code and have users bear the brunt of the impact.

          I prefer to have it with Giraph directly. Anyone else?

          +1. If these connectors should exist (and I think they should), they should work all the time and be maintained. The best way to ensure this is to host them inside one or the other project and since Giraph would sit above HBase (or MR), we should host them. This way the connectors get tested all the time with the rest of the code. If there comes a time when we don't have the ability or support to keep them maintained, then I'd recommend just deleting them entirely from the tree, on the assumption that releasing poorly maintained, non-compatible or buggy code is worse than no code at all. Of course, I doubt this will happen and instead expect we'll always have a volunteer with hbase/accumulo knowledge to keep the code up to date.

          Show
          Jakob Homan added a comment - Per Keith Turner's comments in HAMA-153 would it make more sense to host this submodule on github? I've spent lots of time doing this with the Avro connector for Hive and wish I hadn't. It's quite easy for the connector code to drift from the main code and have users bear the brunt of the impact. I prefer to have it with Giraph directly. Anyone else? +1. If these connectors should exist (and I think they should), they should work all the time and be maintained. The best way to ensure this is to host them inside one or the other project and since Giraph would sit above HBase (or MR), we should host them. This way the connectors get tested all the time with the rest of the code. If there comes a time when we don't have the ability or support to keep them maintained, then I'd recommend just deleting them entirely from the tree, on the assumption that releasing poorly maintained, non-compatible or buggy code is worse than no code at all. Of course, I doubt this will happen and instead expect we'll always have a volunteer with hbase/accumulo knowledge to keep the code up to date.
          Hide
          Brian Femiano added a comment -
          we'll always have a volunteer with hbase/accumulo knowledge to keep the code up to date

          I will gladly do that for the foreseeable future, should this patch get accepted into Giraph.

          Show
          Brian Femiano added a comment - we'll always have a volunteer with hbase/accumulo knowledge to keep the code up to date I will gladly do that for the foreseeable future, should this patch get accepted into Giraph.
          Hide
          Avery Ching added a comment -

          I'll take a look at this patch tonight Brian. =)

          Show
          Avery Ching added a comment - I'll take a look at this patch tonight Brian. =)
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4801/
          -----------------------------------------------------------

          Review request for giraph.

          Summary
          -------

          Brian's patch for GIRAPH-153.

          This addresses bug GIRAPH-153.
          https://issues.apache.org/jira/browse/GIRAPH-153

          Diffs


          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml 1327637

          Diff: https://reviews.apache.org/r/4801/diff

          Testing
          -------

          Thanks,

          Avery

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/ ----------------------------------------------------------- Review request for giraph. Summary ------- Brian's patch for GIRAPH-153 . This addresses bug GIRAPH-153 . https://issues.apache.org/jira/browse/GIRAPH-153 Diffs http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml 1327637 Diff: https://reviews.apache.org/r/4801/diff Testing ------- Thanks, Avery
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4801/#review7020
          -----------------------------------------------------------

          Brian, this kind of stuff will really help lower the bar for using Giraph. Great work! Probably my main comment is that we should get this code to use the same checkstyle.xml as the parent pom.xml. Making that change will likely take you a little bit of time. Once that gets worked out, we should get this committed. I definitely intend to use your same approach to adding the Hive formats as well.

          In response to your comments.

          1) Users must 'mvn install' the giraph artifact in their local repo, at least until we get something posted on maven central.

          Agreed. This isn't too bad, but as you mentioned, a README would be useful. Or you could edit https://cwiki.apache.org/confluence/display/GIRAPH/, which would be possibly easier than maintaining documentation here.

          2) I modified the pom.xml to exclude the artifact from the rat plugin. I realize this is less than desirable, but I couldn't get anything running
          despite numerous attempts at fixing the "too many unapproved licenses" issues. I'm interested to hear your guys thoughts.

          I'm confused why there were too many unapproved licenses. This is probably needed though, this the code has to have the Apache license I believe.

          3) Duplicate BspCase in my submodule, at least until Giraph has a test artifact.

          Yeah, this sucks, but at least you noted it in the comment.

          4) Initializing the AccumuloVertexInputFormat has some procedural limitations inherent in the format design when run with the GiraphJob. It really expects to have control of the Job instance. These can be difficult to track down. I tried to document these in my unit tests and provide some simple error wrapping to help notify users when they see these.

          Not an Accumulo expert, so.....okay. =)

          5) No README.txt or any wiki entry yet. I figured I'd wait and see what feedback you guys had.

          See my response to 1).

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml
          <https://reviews.apache.org/r/4801/#comment15593>

          Brian, I've noticed that there is no usage of the checkstyle plugin, like the parent pom.xml uses (i.e.

          <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-checkstyle-plugin</artifactId>
          <version>2.9</version>
          <configuration>
          <configLocation>checkstyle.xml</configLocation>
          <enableRulesSummary>false</enableRulesSummary>
          <headerLocation>license-header.txt</headerLocation>
          <failOnError>true</failOnError>
          <includeTestSourceDirectory>false</includeTestSourceDirectory>
          </configuration>
          <executions>
          <execution>
          <phase>verify</phase>
          <goals>
          <goal>check</goal>
          </goals>
          </execution>
          </executions>
          </plugin>

          This would be good to have code uniformity.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15594>

          I'm specifying a few examples where the checkstyle.xml will catch you. Here you would need to add javadoc for your members.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15595>

          @Override

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15596>

          Extra *

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15597>

          javadoc

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15598>

          @Override

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment15599>

          if (e

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java
          <https://reviews.apache.org/r/4801/#comment15600>

          extra lines

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml
          <https://reviews.apache.org/r/4801/#comment15592>

          Would be great to get Apache Rat working with giraph-formats-contrib so that we don't forget any licenses here.

          • Avery

          On 2012-04-19 07:27:14, Avery Ching wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4801/

          -----------------------------------------------------------

          (Updated 2012-04-19 07:27:14)

          Review request for giraph.

          Summary

          -------

          Brian's patch for GIRAPH-153.

          This addresses bug GIRAPH-153.

          https://issues.apache.org/jira/browse/GIRAPH-153

          Diffs

          -----

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml 1327637

          Diff: https://reviews.apache.org/r/4801/diff

          Testing

          -------

          Thanks,

          Avery

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/#review7020 ----------------------------------------------------------- Brian, this kind of stuff will really help lower the bar for using Giraph. Great work! Probably my main comment is that we should get this code to use the same checkstyle.xml as the parent pom.xml. Making that change will likely take you a little bit of time. Once that gets worked out, we should get this committed. I definitely intend to use your same approach to adding the Hive formats as well. In response to your comments. 1) Users must 'mvn install' the giraph artifact in their local repo, at least until we get something posted on maven central. Agreed. This isn't too bad, but as you mentioned, a README would be useful. Or you could edit https://cwiki.apache.org/confluence/display/GIRAPH/ , which would be possibly easier than maintaining documentation here. 2) I modified the pom.xml to exclude the artifact from the rat plugin. I realize this is less than desirable, but I couldn't get anything running despite numerous attempts at fixing the "too many unapproved licenses" issues. I'm interested to hear your guys thoughts. I'm confused why there were too many unapproved licenses. This is probably needed though, this the code has to have the Apache license I believe. 3) Duplicate BspCase in my submodule, at least until Giraph has a test artifact. Yeah, this sucks, but at least you noted it in the comment. 4) Initializing the AccumuloVertexInputFormat has some procedural limitations inherent in the format design when run with the GiraphJob. It really expects to have control of the Job instance. These can be difficult to track down. I tried to document these in my unit tests and provide some simple error wrapping to help notify users when they see these. Not an Accumulo expert, so.....okay. =) 5) No README.txt or any wiki entry yet. I figured I'd wait and see what feedback you guys had. See my response to 1). http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml < https://reviews.apache.org/r/4801/#comment15593 > Brian, I've noticed that there is no usage of the checkstyle plugin, like the parent pom.xml uses (i.e. <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-checkstyle-plugin</artifactId> <version>2.9</version> <configuration> <configLocation>checkstyle.xml</configLocation> <enableRulesSummary>false</enableRulesSummary> <headerLocation>license-header.txt</headerLocation> <failOnError>true</failOnError> <includeTestSourceDirectory>false</includeTestSourceDirectory> </configuration> <executions> <execution> <phase>verify</phase> <goals> <goal>check</goal> </goals> </execution> </executions> </plugin> This would be good to have code uniformity. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15594 > I'm specifying a few examples where the checkstyle.xml will catch you. Here you would need to add javadoc for your members. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15595 > @Override http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15596 > Extra * http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15597 > javadoc http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15598 > @Override http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment15599 > if (e http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java < https://reviews.apache.org/r/4801/#comment15600 > extra lines http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml < https://reviews.apache.org/r/4801/#comment15592 > Would be great to get Apache Rat working with giraph-formats-contrib so that we don't forget any licenses here. Avery On 2012-04-19 07:27:14, Avery Ching wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/ ----------------------------------------------------------- (Updated 2012-04-19 07:27:14) Review request for giraph. Summary ------- Brian's patch for GIRAPH-153 . This addresses bug GIRAPH-153 . https://issues.apache.org/jira/browse/GIRAPH-153 Diffs ----- http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml 1327637 Diff: https://reviews.apache.org/r/4801/diff Testing ------- Thanks, Avery
          Hide
          Avery Ching added a comment -

          Okay, so it's still tonight (even though it is 12:44 AM). =)

          Brian, I've done an initial look at the code on reviewboard https://reviews.apache.org/r/4801/. Please take a look. Thanks.

          Show
          Avery Ching added a comment - Okay, so it's still tonight (even though it is 12:44 AM). =) Brian, I've done an initial look at the code on reviewboard https://reviews.apache.org/r/4801/ . Please take a look. Thanks.
          Hide
          Brian Femiano added a comment -

          Sounds good Avery. I'll add the checkstyle plugin and work on cleaning everything up a bit more.

          The rat integration could take me a little longer to troubleshoot. I thought I had included all the proper licenses, but maybe not.

          Show
          Brian Femiano added a comment - Sounds good Avery. I'll add the checkstyle plugin and work on cleaning everything up a bit more. The rat integration could take me a little longer to troubleshoot. I thought I had included all the proper licenses, but maybe not.
          Hide
          Brian Femiano added a comment -

          Added patch GIRAPH-153.1.patch which has the module passing the project rat and checkstyle operations.

          Added, https://cwiki.apache.org/confluence/display/GIRAPH/Giraph+formats+contrib to help users gain familiarity.

          You'll notice this patch has no changes to the parent pom.xml. I didn't have to make any rat exclusions once I added the proper license headers.

          Thank you Avery and Jakob for your continued support in this.

          Show
          Brian Femiano added a comment - Added patch GIRAPH-153 .1.patch which has the module passing the project rat and checkstyle operations. Added, https://cwiki.apache.org/confluence/display/GIRAPH/Giraph+formats+contrib to help users gain familiarity. You'll notice this patch has no changes to the parent pom.xml. I didn't have to make any rat exclusions once I added the proper license headers. Thank you Avery and Jakob for your continued support in this.
          Hide
          Brian Femiano added a comment -

          Updated contrib confluence wiki entry for clarity.

          Show
          Brian Femiano added a comment - Updated contrib confluence wiki entry for clarity.
          Hide
          Brian Femiano added a comment -

          Could a committer please review the .1 patch?

          Thanks.

          Show
          Brian Femiano added a comment - Could a committer please review the .1 patch? Thanks.
          Hide
          Avery Ching added a comment -

          I'll take a look, sorry for the delay.

          Show
          Avery Ching added a comment - I'll take a look, sorry for the delay.
          Hide
          Avery Ching added a comment -

          Brian, I'm having some trouble with your patch. I used a freshly checked out version of giraph to confirm:

          aching@sdwilshmbp13:~/Avery/source/giraph_trunk$ patch -p0 < ~/Desktop/GIRAPH-153.1.patch
          patching file giraph-formats-contrib/LICENSE.txt
          patching file giraph-formats-contrib/license-header.txt
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java
          patching file giraph-formats-contrib/src/main/assembly/compile.xml
          can't find file to patch at input line 1301
          Perhaps you used the wrong -p or --strip option?
          The text leading up to this was:
          --------------------------

          Index: giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java
          ===================================================================
          — giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java (revision 0)
          +++ giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java (working copy)
          --------------------------
          File to patch:
          Show
          Avery Ching added a comment - Brian, I'm having some trouble with your patch. I used a freshly checked out version of giraph to confirm: aching@sdwilshmbp13:~/Avery/source/giraph_trunk$ patch -p0 < ~/Desktop/ GIRAPH-153 .1.patch patching file giraph-formats-contrib/LICENSE.txt patching file giraph-formats-contrib/license-header.txt patching file giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java patching file giraph-formats-contrib/src/main/assembly/compile.xml can't find file to patch at input line 1301 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- Index: giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java =================================================================== — giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java (revision 0) +++ giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java (working copy) -------------------------- File to patch:
          Hide
          Brian Femiano added a comment -

          Hmm I can't reproduce here. Does the below look fine?

          pfi-co2fvo4vdf91:trunk bfemiano$ patch -p0 < GIRAPH-153.1.patch
          patching file giraph-formats-contrib/LICENSE.txt
          patching file giraph-formats-contrib/license-header.txt
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java
          patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java
          patching file giraph-formats-contrib/src/main/assembly/compile.xml
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java
          Reversed (or previously applied) patch detected! Assume -R? [n] y
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java
          patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java
          Reversed (or previously applied) patch detected! Assume -R? [n] y
          patching file giraph-formats-contrib/pom.xml

          Show
          Brian Femiano added a comment - Hmm I can't reproduce here. Does the below look fine? pfi-co2fvo4vdf91:trunk bfemiano$ patch -p0 < GIRAPH-153 .1.patch patching file giraph-formats-contrib/LICENSE.txt patching file giraph-formats-contrib/license-header.txt patching file giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java patching file giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java patching file giraph-formats-contrib/src/main/assembly/compile.xml patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java Reversed (or previously applied) patch detected! Assume -R? [n] y patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java Reversed (or previously applied) patch detected! Assume -R? [n] y patching file giraph-formats-contrib/pom.xml
          Hide
          Avery Ching added a comment -

          Is this a fresh checkout? We shouldn't have to answer any questions like "Reversed (or previously applied) patch detected! Assume -R".

          Show
          Avery Ching added a comment - Is this a fresh checkout? We shouldn't have to answer any questions like "Reversed (or previously applied) patch detected! Assume -R".
          Hide
          Brian Femiano added a comment -

          Ahhh yea I see what I did now. I changed those files while they were added, but before committed. It screwed up the patch.

          GIRAPH-153.2.patch passes the trunk 'mvn verify'. It should also patch cleanly on a fresh checkout.

          Sorry for the confusion Avery.

          Show
          Brian Femiano added a comment - Ahhh yea I see what I did now. I changed those files while they were added, but before committed. It screwed up the patch. GIRAPH-153 .2.patch passes the trunk 'mvn verify'. It should also patch cleanly on a fresh checkout. Sorry for the confusion Avery.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4801/
          -----------------------------------------------------------

          (Updated 2012-04-30 23:54:55.758151)

          Review request for giraph.

          Changes
          -------

          Update of Brian's 153.2.

          Summary
          -------

          Brian's patch for GIRAPH-153.

          This addresses bug GIRAPH-153.
          https://issues.apache.org/jira/browse/GIRAPH-153

          Diffs (updated)


          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION
          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4801/diff

          Testing
          -------

          Thanks,

          Avery

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/ ----------------------------------------------------------- (Updated 2012-04-30 23:54:55.758151) Review request for giraph. Changes ------- Update of Brian's 153.2. Summary ------- Brian's patch for GIRAPH-153 . This addresses bug GIRAPH-153 . https://issues.apache.org/jira/browse/GIRAPH-153 Diffs (updated) http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION Diff: https://reviews.apache.org/r/4801/diff Testing ------- Thanks, Avery
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4801/#review7404
          -----------------------------------------------------------

          Hi Brian, the patch applies nicely, but it filled with duplicates. Also there are some javadoc indentation fixes to make. I just gave a couple of examples. Can you please fix this and resubmit? Thanks!

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt
          <https://reviews.apache.org/r/4801/#comment16328>

          This license is duplicated several times.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt
          <https://reviews.apache.org/r/4801/#comment16329>

          This license is duplicated several times.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml
          <https://reviews.apache.org/r/4801/#comment16330>

          More duplication.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16331>

          Please fix indentation.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16332>

          Please fix indentation.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16333>

          Please fix indentation.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16334>

          Please fix indentation.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16335>

          Code duplication.

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          <https://reviews.apache.org/r/4801/#comment16336>

          Extra *

          • Avery

          On 2012-04-30 23:54:55, Avery Ching wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4801/

          -----------------------------------------------------------

          (Updated 2012-04-30 23:54:55)

          Review request for giraph.

          Summary

          -------

          Brian's patch for GIRAPH-153.

          This addresses bug GIRAPH-153.

          https://issues.apache.org/jira/browse/GIRAPH-153

          Diffs

          -----

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION

          http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4801/diff

          Testing

          -------

          Thanks,

          Avery

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/#review7404 ----------------------------------------------------------- Hi Brian, the patch applies nicely, but it filled with duplicates. Also there are some javadoc indentation fixes to make. I just gave a couple of examples. Can you please fix this and resubmit? Thanks! http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt < https://reviews.apache.org/r/4801/#comment16328 > This license is duplicated several times. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt < https://reviews.apache.org/r/4801/#comment16329 > This license is duplicated several times. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml < https://reviews.apache.org/r/4801/#comment16330 > More duplication. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16331 > Please fix indentation. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16332 > Please fix indentation. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16333 > Please fix indentation. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16334 > Please fix indentation. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16335 > Code duplication. http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java < https://reviews.apache.org/r/4801/#comment16336 > Extra * Avery On 2012-04-30 23:54:55, Avery Ching wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4801/ ----------------------------------------------------------- (Updated 2012-04-30 23:54:55) Review request for giraph. Summary ------- Brian's patch for GIRAPH-153 . This addresses bug GIRAPH-153 . https://issues.apache.org/jira/browse/GIRAPH-153 Diffs ----- http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/LICENSE.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/license-header.txt PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/pom.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java PRE-CREATION http://svn.apache.org/repos/asf/incubator/giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java PRE-CREATION Diff: https://reviews.apache.org/r/4801/diff Testing ------- Thanks, Avery
          Hide
          Brian Femiano added a comment -

          Wow no idea how that happened. Standby.

          Show
          Brian Femiano added a comment - Wow no idea how that happened. Standby.
          Hide
          Avery Ching added a comment -

          No problem. The red flag for me was that this patch (244K) was so much bigger than the previous one (85k).

          Show
          Avery Ching added a comment - No problem. The red flag for me was that this patch (244K) was so much bigger than the previous one (85k).
          Hide
          Brian Femiano added a comment -

          Patch GIRAPH-153.3.patch fixes the javadoc indent issues not appearing in checkstyle. I also removed all the duplication and redundant LICENSE.txt and license-header.txt

          Show
          Brian Femiano added a comment - Patch GIRAPH-153 .3.patch fixes the javadoc indent issues not appearing in checkstyle. I also removed all the duplication and redundant LICENSE.txt and license-header.txt
          Hide
          Brian Femiano added a comment -

          Avery, any luck with the patch? This patch should be ok.

          Show
          Brian Femiano added a comment - Avery, any luck with the patch? This patch should be ok.
          Hide
          Avery Ching added a comment -

          I'll take a look this weekend Brian. Thanks for the reminder.

          Show
          Avery Ching added a comment - I'll take a look this weekend Brian. Thanks for the reminder.
          Hide
          Brian Femiano added a comment -

          I'll take down the confluence entry until this is approved. We don't want anyone getting the wrong idea.

          Show
          Brian Femiano added a comment - I'll take down the confluence entry until this is approved. We don't want anyone getting the wrong idea.
          Hide
          Avery Ching added a comment -

          +1, nice job Brian. Sorry it took me longer than I expected to get to this.

          Show
          Avery Ching added a comment - +1, nice job Brian. Sorry it took me longer than I expected to get to this.
          Hide
          Hudson added a comment -

          Integrated in Giraph-trunk-Commit #112 (See https://builds.apache.org/job/Giraph-trunk-Commit/112/)
          GIRAPH-153: HBase/Accumulo Input and Output formats. (bfem via aching) (Revision 1344020)

          Result = SUCCESS
          aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344020
          Files :

          • /giraph/trunk/CHANGELOG
          • /giraph/trunk/giraph-formats-contrib
          • /giraph/trunk/giraph-formats-contrib/pom.xml
          • /giraph/trunk/giraph-formats-contrib/src
          • /giraph/trunk/giraph-formats-contrib/src/main
          • /giraph/trunk/giraph-formats-contrib/src/main/assembly
          • /giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml
          • /giraph/trunk/giraph-formats-contrib/src/main/java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java
          • /giraph/trunk/giraph-formats-contrib/src/test
          • /giraph/trunk/giraph-formats-contrib/src/test/java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java
          • /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java
          Show
          Hudson added a comment - Integrated in Giraph-trunk-Commit #112 (See https://builds.apache.org/job/Giraph-trunk-Commit/112/ ) GIRAPH-153 : HBase/Accumulo Input and Output formats. (bfem via aching) (Revision 1344020) Result = SUCCESS aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344020 Files : /giraph/trunk/CHANGELOG /giraph/trunk/giraph-formats-contrib /giraph/trunk/giraph-formats-contrib/pom.xml /giraph/trunk/giraph-formats-contrib/src /giraph/trunk/giraph-formats-contrib/src/main /giraph/trunk/giraph-formats-contrib/src/main/assembly /giraph/trunk/giraph-formats-contrib/src/main/assembly/compile.xml /giraph/trunk/giraph-formats-contrib/src/main/java /giraph/trunk/giraph-formats-contrib/src/main/java/org /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/package-info.java /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexInputFormat.java /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/HBaseVertexOutputFormat.java /giraph/trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/format/hbase/package-info.java /giraph/trunk/giraph-formats-contrib/src/test /giraph/trunk/giraph-formats-contrib/src/test/java /giraph/trunk/giraph-formats-contrib/src/test/java/org /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/BspCase.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/TestAccumuloVertexFormat.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeInputFormat.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/accumulo/edgemarker/AccumuloEdgeOutputFormat.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/TestHBaseRootMarkerVertextFormat.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeInputFormat.java /giraph/trunk/giraph-formats-contrib/src/test/java/org/apache/giraph/format/hbase/edgemarker/TableEdgeOutputFormat.java
          Hide
          Avery Ching added a comment -

          Hudson reports success, closing.

          Show
          Avery Ching added a comment - Hudson reports success, closing.
          Hide
          Paolo Castagna added a comment -

          pom.xml for giraph-formats-contrib has <groupId>giraph</groupId>, shouldn't that be <groupId>org.apache.giraph</groupId>?

          Show
          Paolo Castagna added a comment - pom.xml for giraph-formats-contrib has <groupId>giraph</groupId>, shouldn't that be <groupId>org.apache.giraph</groupId>?
          Hide
          Brian Femiano added a comment -

          I can change it to whatever the Giraph team decides for the maven central artifact.

          org.apache.giraph is probably correct.

          Show
          Brian Femiano added a comment - I can change it to whatever the Giraph team decides for the maven central artifact. org.apache.giraph is probably correct.
          Hide
          Paolo Castagna added a comment -

          Also, src/main/assembly/compile.xml ... is it used? It has duplicates <assembly> tags in it. It's probably not a big problem, but since I was passing by, I noticed. (By the way, nice stuff the integration with HBase and Accumulo )

          Show
          Paolo Castagna added a comment - Also, src/main/assembly/compile.xml ... is it used? It has duplicates <assembly> tags in it. It's probably not a big problem, but since I was passing by, I noticed. (By the way, nice stuff the integration with HBase and Accumulo )
          Hide
          Brian Femiano added a comment -

          compile.xml exists in case there are compile-time exclusions required from the fat jar. Currently there are no exclusions, but this may not always be the case. Trunk currently has compile.xml as well. Before I added that, the project built with the default fat jar, and included everything. The duplicate assembly tags aren't necessary. When I get back to my workstation next week I'll fix the assembly tags and the maven groupid.

          See GIRAPH-159.

          Show
          Brian Femiano added a comment - compile.xml exists in case there are compile-time exclusions required from the fat jar. Currently there are no exclusions, but this may not always be the case. Trunk currently has compile.xml as well. Before I added that, the project built with the default fat jar, and included everything. The duplicate assembly tags aren't necessary. When I get back to my workstation next week I'll fix the assembly tags and the maven groupid. See GIRAPH-159 .
          Hide
          Claudio Martella added a comment -

          this is very cool contribution. thanks

          Show
          Claudio Martella added a comment - this is very cool contribution. thanks

            People

            • Assignee:
              Brian Femiano
              Reporter:
              Brian Femiano
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development