You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Sean Busbey <se...@manvsbeard.com> on 2014/04/09 23:28:04 UTC

Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

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

Review request for accumulo and Josh Elser.


Bugs: ACCUMULO-2654
    https://issues.apache.org/jira/browse/ACCUMULO-2654


Repository: accumulo


Description
-------

Adds a simple utility for creating an empty RFile, leveraging existing code.


Diffs
-----

  src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
  src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 

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


Testing
-------

tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.


Thanks,

Sean Busbey


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 15, 2014, 7:51 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 84
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line84>
> >
> >     If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file
> 
> Sean Busbey wrote:
>     so a scan of the metadata table for the filename returning something would be sufficient?
>     
>     FWIW, I cribbed this from another test that uses printinfo to validate locality groups got properly serialized.
> 
> kturner wrote:
>     Thinking about this some more, Accumulo generates unique filenames for every file (even across tables and tablets). So F0000000.rf would only be used if the metadata table had not compacted :)    Should probably get the filename from the metadata table.

I believe this an other tests rely on the fact that the test cluster has just been set up and used exclusively for the initial data load. Fixing this assumption will be a fair bit of additional work. Could we do it as a follow on in 1.6+ only?


- Sean


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 15, 2014, 7:51 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 84
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line84>
> >
> >     If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file

so a scan of the metadata table for the filename returning something would be sufficient?

FWIW, I cribbed this from another test that uses printinfo to validate locality groups got properly serialized.


- Sean


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.

> On April 15, 2014, 7:51 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 84
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line84>
> >
> >     If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file
> 
> Sean Busbey wrote:
>     so a scan of the metadata table for the filename returning something would be sufficient?
>     
>     FWIW, I cribbed this from another test that uses printinfo to validate locality groups got properly serialized.
> 
> kturner wrote:
>     Thinking about this some more, Accumulo generates unique filenames for every file (even across tables and tablets). So F0000000.rf would only be used if the metadata table had not compacted :)    Should probably get the filename from the metadata table.
> 
> Sean Busbey wrote:
>     I believe this an other tests rely on the fact that the test cluster has just been set up and used exclusively for the initial data load. Fixing this assumption will be a fair bit of additional work. Could we do it as a follow on in 1.6+ only?

thats ok w/ me if the hadoop fs -rm does cause the test to fail when the file does not exist


- kturner


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.

> On April 15, 2014, 7:51 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 84
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line84>
> >
> >     If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file
> 
> Sean Busbey wrote:
>     so a scan of the metadata table for the filename returning something would be sufficient?
>     
>     FWIW, I cribbed this from another test that uses printinfo to validate locality groups got properly serialized.

Thinking about this some more, Accumulo generates unique filenames for every file (even across tables and tablets). So F0000000.rf would only be used if the metadata table had not compacted :)    Should probably get the filename from the metadata table.


- kturner


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 15, 2014, 7:51 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 84
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line84>
> >
> >     If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file
> 
> Sean Busbey wrote:
>     so a scan of the metadata table for the filename returning something would be sufficient?
>     
>     FWIW, I cribbed this from another test that uses printinfo to validate locality groups got properly serialized.
> 
> kturner wrote:
>     Thinking about this some more, Accumulo generates unique filenames for every file (even across tables and tablets). So F0000000.rf would only be used if the metadata table had not compacted :)    Should probably get the filename from the metadata table.
> 
> Sean Busbey wrote:
>     I believe this an other tests rely on the fact that the test cluster has just been set up and used exclusively for the initial data load. Fixing this assumption will be a fair bit of additional work. Could we do it as a follow on in 1.6+ only?
> 
> kturner wrote:
>     thats ok w/ me if the hadoop fs -rm does cause the test to fail when the file does not exist

Turns out the 1.6+ version of these tests already do the lookup.


- Sean


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


On April 15, 2014, 8:51 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 8:51 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40448
-----------------------------------------------------------



test/system/auto/simple/recoverWithEmpty.py
<https://reviews.apache.org/r/20180/#comment73466>

    If this file exist but it not referenced in the metadata table, then this test will not properly validate what its trying to... maybe the test should look in the metadata table to get the filename, or to just verify metadata refs the file


- kturner


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40550
-----------------------------------------------------------

Ship it!


Ship It!

- Josh Elser


On April 15, 2014, 8:51 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 8:51 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40463
-----------------------------------------------------------

Ship it!


Ship It!

- kturner


On April 15, 2014, 8:51 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 8:51 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/
-----------------------------------------------------------

(Updated April 15, 2014, 8:51 p.m.)


Review request for accumulo and Josh Elser.


Changes
-------

updated with feedback from Keith about checking our assumption about backing files.


Bugs: ACCUMULO-2654
    https://issues.apache.org/jira/browse/ACCUMULO-2654


Repository: accumulo


Description
-------

Adds a simple utility for creating an empty RFile, leveraging existing code.


Diffs (updated)
-----

  src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
  src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
  test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 

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


Testing
-------

tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.

ran new functional test.


Thanks,

Sean Busbey


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 15, 2014, 7:47 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 86
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line86>
> >
> >     will this cuase the test to fail if the file does not exist?

yes, I presumed that was desirable?


- Sean


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.

> On April 15, 2014, 7:47 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 86
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line86>
> >
> >     will this cuase the test to fail if the file does not exist?
> 
> Sean Busbey wrote:
>     yes, I presumed that was desirable?

yes, want the test to fail if the file does not exist.... but I am not sure about hadoop return codes, I am also not sure if waitForStop checks return codes....

but this may be moot if the metadata table is scanned to find the filename...


- kturner


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 15, 2014, 7:47 p.m., kturner wrote:
> > test/system/auto/simple/recoverWithEmpty.py, line 86
> > <https://reviews.apache.org/r/20180/diff/3/?file=558311#file558311line86>
> >
> >     will this cuase the test to fail if the file does not exist?
> 
> Sean Busbey wrote:
>     yes, I presumed that was desirable?
> 
> kturner wrote:
>     yes, want the test to fail if the file does not exist.... but I am not sure about hadoop return codes, I am also not sure if waitForStop checks return codes....
>     
>     but this may be moot if the metadata table is scanned to find the filename...

I confirmed that hadoop properly returns non-0 if the file doesn't exist, runOn returns it, and waitForStop checks it.


- Sean


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


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40447
-----------------------------------------------------------



test/system/auto/simple/recoverWithEmpty.py
<https://reviews.apache.org/r/20180/#comment73463>

    will this cuase the test to fail if the file does not exist?


- kturner


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40455
-----------------------------------------------------------



test/system/auto/simple/recoverWithEmpty.py
<https://reviews.apache.org/r/20180/#comment73475>

    If this ensured the scan returned nothing it would make the test more robust


- kturner


On April 15, 2014, 7:38 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 15, 2014, 7:38 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
>   test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> ran new functional test.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/
-----------------------------------------------------------

(Updated April 15, 2014, 7:38 p.m.)


Review request for accumulo and Josh Elser.


Bugs: ACCUMULO-2654
    https://issues.apache.org/jira/browse/ACCUMULO-2654


Repository: accumulo


Description
-------

Adds a simple utility for creating an empty RFile, leveraging existing code.


Diffs
-----

  src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
  src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
  test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 

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


Testing (updated)
-------

tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.

ran new functional test.


Thanks,

Sean Busbey


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/
-----------------------------------------------------------

(Updated April 15, 2014, 7:21 p.m.)


Review request for accumulo and Josh Elser.


Changes
-------

includes feedback from Vikram, Mike, Josh, and Keith. adds functional test.


Bugs: ACCUMULO-2654
    https://issues.apache.org/jira/browse/ACCUMULO-2654


Repository: accumulo


Description
-------

Adds a simple utility for creating an empty RFile, leveraging existing code.


Diffs (updated)
-----

  src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
  src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
  test/system/auto/simple/recoverWithEmpty.py PRE-CREATION 

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


Testing
-------

tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.


Thanks,

Sean Busbey


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Vikram Srivastava <vi...@gmail.com>.

> On April 9, 2014, 9:37 p.m., Vikram Srivastava wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, lines 59-66
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line59>
> >
> >     We can first validate all args, and then create them in 2nd pass to ensure some files don't get created.
> 
> Sean Busbey wrote:
>     Worth switching to Guava's Precondition? core doesn't have a Guava dep in this branch.

I'll defer to you to use Precondition here. My concern was only that if user enters "f1.rf f2.rf f3.nrf", it's more user friendly to give error in the beginning rather than aborting with an error in the middle.


- Vikram


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 9, 2014, 9:37 p.m., Vikram Srivastava wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, lines 59-66
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line59>
> >
> >     We can first validate all args, and then create them in 2nd pass to ensure some files don't get created.

Worth switching to Guava's Precondition? core doesn't have a Guava dep in this branch.


- Sean


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Vikram Srivastava <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review39937
-----------------------------------------------------------

Ship it!


LGTM. One minor comment regarding usability.


src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java
<https://reviews.apache.org/r/20180/#comment72704>

    We can first validate all args, and then create them in 2nd pass to ensure some files don't get created.


- Vikram Srivastava


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Josh Elser <jo...@gmail.com>.

> On April 10, 2014, 2 a.m., Josh Elser wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, line 43
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line43>
> >
> >     This will create a file in HDFS because CachedConfiguration will include the HDFS conf files too, right? It would be good to note where the empty rfile will be created. Beware of multi-volume changes in the 1.6 merge too
> 
> Sean Busbey wrote:
>     CachedConfiguration is just about Hadoop configs, AFAICT. It behaves per normal HDFS client rules: it uses the default filesystem if you don't give it a full url. On most hdfs installs that's storing into HDFS. I also tested making it with a local filesystem.
>     
>     Do you mean note like with a log message?
>     
>     I figured in 1.6, it's better to keep things simple and output with normal HDFS apis. This will normally be used in response to an error and the missing file will be in a particular HDFS instance.
> 
> Josh Elser wrote:
>     I think the help message would be best. If I were using a tool, that'd be the first place I check for more information about what said tool is doing.
> 
> Sean Busbey wrote:
>     Oh! you mean that the "path" component is a FileSystem URL and will default to the one in your Hadoop configs.
>     
>     I'll have to see if commons-cli will let me put a description of unparsed args. Would it be terrible if that part of the help message could only be on the later branches when we have jcommander?

Bingo -- not the end of the world if commons-cli variant doesn't do something. Maybe a log message about the Path that's being created for those cases? Your judgement is sufficient.


- Josh


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Josh Elser <jo...@gmail.com>.

> On April 10, 2014, 2 a.m., Josh Elser wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, line 43
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line43>
> >
> >     This will create a file in HDFS because CachedConfiguration will include the HDFS conf files too, right? It would be good to note where the empty rfile will be created. Beware of multi-volume changes in the 1.6 merge too
> 
> Sean Busbey wrote:
>     CachedConfiguration is just about Hadoop configs, AFAICT. It behaves per normal HDFS client rules: it uses the default filesystem if you don't give it a full url. On most hdfs installs that's storing into HDFS. I also tested making it with a local filesystem.
>     
>     Do you mean note like with a log message?
>     
>     I figured in 1.6, it's better to keep things simple and output with normal HDFS apis. This will normally be used in response to an error and the missing file will be in a particular HDFS instance.

I think the help message would be best. If I were using a tool, that'd be the first place I check for more information about what said tool is doing.


- Josh


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 10, 2014, 2 a.m., Josh Elser wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, line 43
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line43>
> >
> >     This will create a file in HDFS because CachedConfiguration will include the HDFS conf files too, right? It would be good to note where the empty rfile will be created. Beware of multi-volume changes in the 1.6 merge too

CachedConfiguration is just about Hadoop configs, AFAICT. It behaves per normal HDFS client rules: it uses the default filesystem if you don't give it a full url. On most hdfs installs that's storing into HDFS. I also tested making it with a local filesystem.

Do you mean note like with a log message?

I figured in 1.6, it's better to keep things simple and output with normal HDFS apis. This will normally be used in response to an error and the missing file will be in a particular HDFS instance.


- Sean


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 10, 2014, 2 a.m., Josh Elser wrote:
> > src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java, line 43
> > <https://reviews.apache.org/r/20180/diff/2/?file=554090#file554090line43>
> >
> >     This will create a file in HDFS because CachedConfiguration will include the HDFS conf files too, right? It would be good to note where the empty rfile will be created. Beware of multi-volume changes in the 1.6 merge too
> 
> Sean Busbey wrote:
>     CachedConfiguration is just about Hadoop configs, AFAICT. It behaves per normal HDFS client rules: it uses the default filesystem if you don't give it a full url. On most hdfs installs that's storing into HDFS. I also tested making it with a local filesystem.
>     
>     Do you mean note like with a log message?
>     
>     I figured in 1.6, it's better to keep things simple and output with normal HDFS apis. This will normally be used in response to an error and the missing file will be in a particular HDFS instance.
> 
> Josh Elser wrote:
>     I think the help message would be best. If I were using a tool, that'd be the first place I check for more information about what said tool is doing.

Oh! you mean that the "path" component is a FileSystem URL and will default to the one in your Hadoop configs.

I'll have to see if commons-cli will let me put a description of unparsed args. Would it be terrible if that part of the help message could only be on the later branches when we have jcommander?


- Sean


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review39975
-----------------------------------------------------------



src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java
<https://reviews.apache.org/r/20180/#comment72758>

    This will create a file in HDFS because CachedConfiguration will include the HDFS conf files too, right? It would be good to note where the empty rfile will be created. Beware of multi-volume changes in the 1.6 merge too


- Josh Elser


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review40021
-----------------------------------------------------------



src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java
<https://reviews.apache.org/r/20180/#comment72806>

    Could use Path.getFileSystem() in loop.  This will help w/ case where user specifies fully qualified URI.  Thought of this after reading Josh's comments.


- kturner


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 9, 2014, 9:42 p.m., Mike Drob wrote:
> > Worth noting that in later version we switched away from commons-cli in favor of jcommander.
> > 
> > Also, can you add a unit test?
> 
> kturner wrote:
>     Since Accumulo does not normally create empty RFiles, its not a well tested thing. The RFile unit test excercises creating an empty rfile and seeking it.  This ensures that an empty rfile is readable.  I am wondering if its worthwhile to have an IT that covers the use case this utility is intended for.   Could have an IT that offlines a table, replaces one of its files with an empty rfile, onlines, and then scans.

re: commons-cli v jcommander, I figured after merging forward I could refactor it to match the system used in later versions. I figured it was better not to introduce a new dep in the older branch.


- Sean


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Sean Busbey <se...@manvsbeard.com>.

> On April 9, 2014, 9:42 p.m., Mike Drob wrote:
> > Worth noting that in later version we switched away from commons-cli in favor of jcommander.
> > 
> > Also, can you add a unit test?
> 
> kturner wrote:
>     Since Accumulo does not normally create empty RFiles, its not a well tested thing. The RFile unit test excercises creating an empty rfile and seeking it.  This ensures that an empty rfile is readable.  I am wondering if its worthwhile to have an IT that covers the use case this utility is intended for.   Could have an IT that offlines a table, replaces one of its files with an empty rfile, onlines, and then scans.
> 
> Sean Busbey wrote:
>     re: commons-cli v jcommander, I figured after merging forward I could refactor it to match the system used in later versions. I figured it was better not to introduce a new dep in the older branch.

What would a unit test cover? I think Keith's idea of a IT is likely to be more useful.

Success criteria is scan completes with no errors? (i.e. does not check the values actually returned)


- Sean


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.

> On April 9, 2014, 9:42 p.m., Mike Drob wrote:
> > Worth noting that in later version we switched away from commons-cli in favor of jcommander.
> > 
> > Also, can you add a unit test?
> 
> kturner wrote:
>     Since Accumulo does not normally create empty RFiles, its not a well tested thing. The RFile unit test excercises creating an empty rfile and seeking it.  This ensures that an empty rfile is readable.  I am wondering if its worthwhile to have an IT that covers the use case this utility is intended for.   Could have an IT that offlines a table, replaces one of its files with an empty rfile, onlines, and then scans.
> 
> Sean Busbey wrote:
>     re: commons-cli v jcommander, I figured after merging forward I could refactor it to match the system used in later versions. I figured it was better not to introduce a new dep in the older branch.
> 
> Sean Busbey wrote:
>     What would a unit test cover? I think Keith's idea of a IT is likely to be more useful.
>     
>     Success criteria is scan completes with no errors? (i.e. does not check the values actually returned)

yeah, thats what I was thinking.  Success is Accumulo not blowing up when scanning an empty file. 


- kturner


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by ke...@deenlo.com.

> On April 9, 2014, 9:42 p.m., Mike Drob wrote:
> > Worth noting that in later version we switched away from commons-cli in favor of jcommander.
> > 
> > Also, can you add a unit test?

Since Accumulo does not normally create empty RFiles, its not a well tested thing. The RFile unit test excercises creating an empty rfile and seeking it.  This ensures that an empty rfile is readable.  I am wondering if its worthwhile to have an IT that covers the use case this utility is intended for.   Could have an IT that offlines a table, replaces one of its files with an empty rfile, onlines, and then scans.


- kturner


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


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>


Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.

Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20180/#review39938
-----------------------------------------------------------


Worth noting that in later version we switched away from commons-cli in favor of jcommander.

Also, can you add a unit test?

- Mike Drob


On April 9, 2014, 9:28 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20180/
> -----------------------------------------------------------
> 
> (Updated April 9, 2014, 9:28 p.m.)
> 
> 
> Review request for accumulo and Josh Elser.
> 
> 
> Bugs: ACCUMULO-2654
>     https://issues.apache.org/jira/browse/ACCUMULO-2654
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a simple utility for creating an empty RFile, leveraging existing code.
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java PRE-CREATION 
>   src/core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java 5374332 
> 
> Diff: https://reviews.apache.org/r/20180/diff/
> 
> 
> Testing
> -------
> 
> tested basic error handling and help messages. tested creating file on hdfs and local file system. tested default codec, gz, and specifying the same codec as default. Used PrintInfo to verify generated files.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>