Re: 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/#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.
--- 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.
--- 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.
--- 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.
--- 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.
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.
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.
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.
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 F000.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.
--- 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.
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.
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 F000.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.
--- 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.
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 F000.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.
--- 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.
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.
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.
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.
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.
--- 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.
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.
--- 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.
--- 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
Re: Review Request 20180: ACCUMULO-2654 Adds utility for creating empty rfile.
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.
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.
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.
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.
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.
--- 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
