This is an automated email from the ASF dual-hosted git repository. jmark99 pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new 35319d4 Move ignoreEmptyDir opt to ImportOptions interface (#2045) 35319d4 is described below commit 35319d441f699178d5facb1b71d4279e6bdfe3e0 Author: Mark Owens <jmar...@apache.org> AuthorDate: Thu Apr 29 11:14:17 2021 -0400 Move ignoreEmptyDir opt to ImportOptions interface (#2045) Update the importdirectory ignoreEmptyDir option to use the bulk import fluent API. The ignoreEmptyDir option is moved into the ImportOptions interface and can be set during the call to the load method. Added @since 2.1.0 tag to ignoreEmptyDir method. --- .../core/client/admin/TableOperations.java | 33 +++++----------------- .../core/clientImpl/TableOperationsImpl.java | 8 +----- .../accumulo/core/clientImpl/bulk/BulkImport.java | 13 +++++---- .../shell/commands/ImportDirectoryCommand.java | 4 +-- .../shell/commands/ImportDirectoryCommandTest.java | 8 +++--- .../apache/accumulo/test/functional/BulkIT.java | 19 +++++++++---- .../apache/accumulo/test/functional/BulkNewIT.java | 17 ++++++++--- 7 files changed, 48 insertions(+), 54 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java b/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java index 78b99fe..c6ac0e9 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java +++ b/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java @@ -717,6 +717,13 @@ public interface TableOperations { ImportMappingOptions tableTime(boolean value); /** + * Ignores empty bulk import source directory, rather than throwing an IllegalArgumentException. + * + * @since 2.1.0 + */ + ImportMappingOptions ignoreEmptyDir(boolean ignore); + + /** * Loads the files into the table. */ void load() @@ -802,32 +809,6 @@ public interface TableOperations { * @since 2.0.0 */ default ImportDestinationArguments importDirectory(String directory) { - return importDirectory(directory, false); - } - - /** - * Bulk import the files in a directory into a table. Files can be created using - * {@link RFile#newWriter()}. - * <p> - * This new method of bulk import examines files in the current process outside of holding a table - * lock. The old bulk import method ({@link #importDirectory(String, String, String, boolean)}) - * examines files on the server side while holding a table read lock. This version of the method - * provides a boolean argument which will prevent an exception from being thrown if the supplied - * directory contains no files. - * <p> - * This API supports adding files to online and offline tables. - * <p> - * For example, to bulk import files from the directory 'dir1' into the table 'table1' use the - * following code. - * - * <pre> - * client.tableOperations().importDirectory("dir1", true).to("table1").load(); - * </pre> - * - * @since 2.0.0 - */ - default ImportDestinationArguments importDirectory(final String directory, - final boolean ignoreEmptyDir) { throw new UnsupportedOperationException(); } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 4b630e2..0c103b3 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -2003,12 +2003,6 @@ public class TableOperationsImpl extends TableOperationsHelper { @Override public ImportDestinationArguments importDirectory(String directory) { - return importDirectory(directory, false); - } - - @Override - public ImportDestinationArguments importDirectory(final String directory, - final boolean ignoreEmptyDir) { - return new BulkImport(directory, ignoreEmptyDir, context); + return new BulkImport(directory, context); } } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java index ec469d2..995b73e 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java @@ -97,24 +97,19 @@ public class BulkImport implements ImportDestinationArguments, ImportMappingOpti private static final Logger log = LoggerFactory.getLogger(BulkImport.class); private boolean setTime = false; + private boolean ignoreEmptyDir = false; private Executor executor = null; private final String dir; private int numThreads = -1; private final ClientContext context; private String tableName; - private boolean ignoreEmptyDir = false; private LoadPlan plan = null; public BulkImport(String directory, ClientContext context) { - this(directory, false, context); - } - - public BulkImport(String directory, boolean ignoreEmptyDirectory, ClientContext context) { this.context = context; this.dir = Objects.requireNonNull(directory); - this.ignoreEmptyDir = ignoreEmptyDirectory; } @Override @@ -124,6 +119,12 @@ public class BulkImport implements ImportDestinationArguments, ImportMappingOpti } @Override + public ImportMappingOptions ignoreEmptyDir(boolean ignore) { + this.ignoreEmptyDir = ignore; + return this; + } + + @Override public void load() throws TableNotFoundException, IOException, AccumuloException, AccumuloSecurityException { diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/ImportDirectoryCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/ImportDirectoryCommand.java index a590c5c..1def577 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/ImportDirectoryCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/ImportDirectoryCommand.java @@ -62,8 +62,8 @@ public class ImportDirectoryCommand extends Command { case 2: { // new bulk import only takes 2 args setTime = Boolean.parseBoolean(cl.getArgs()[1]); - shellState.getAccumuloClient().tableOperations().importDirectory(dir, ignore).to(tableName) - .tableTime(setTime).load(); + shellState.getAccumuloClient().tableOperations().importDirectory(dir).to(tableName) + .tableTime(setTime).ignoreEmptyDir(ignore).load(); break; } case 3: { diff --git a/shell/src/test/java/org/apache/accumulo/shell/commands/ImportDirectoryCommandTest.java b/shell/src/test/java/org/apache/accumulo/shell/commands/ImportDirectoryCommandTest.java index 9b2aca6..1370d03 100644 --- a/shell/src/test/java/org/apache/accumulo/shell/commands/ImportDirectoryCommandTest.java +++ b/shell/src/test/java/org/apache/accumulo/shell/commands/ImportDirectoryCommandTest.java @@ -85,10 +85,10 @@ public class ImportDirectoryCommandTest { shellState.checkTableState(); expectLastCall().once(); - // given the -i option, the ignoreEmptyBulkDir boolean is set to false - expect(tableOperations.importDirectory("in_dir", false)).andReturn(bulkImport).once(); + expect(tableOperations.importDirectory("in_dir")).andReturn(bulkImport).once(); expect(bulkImport.to("tablename")).andReturn(bulkImport).once(); expect(bulkImport.tableTime(false)).andReturn(bulkImport).once(); + expect(bulkImport.ignoreEmptyDir(false)).andReturn(bulkImport).once(); bulkImport.load(); expectLastCall().once(); @@ -119,10 +119,10 @@ public class ImportDirectoryCommandTest { // shellState.checkTableState() is NOT called - // given the -i option, the ignoreEmptyBulkDir boolean is set to true - expect(tableOperations.importDirectory("in_dir", true)).andReturn(bulkImport).once(); + expect(tableOperations.importDirectory("in_dir")).andReturn(bulkImport).once(); expect(bulkImport.to("passedName")).andReturn(bulkImport).once(); expect(bulkImport.tableTime(false)).andReturn(bulkImport).once(); + expect(bulkImport.ignoreEmptyDir(true)).andReturn(bulkImport).once(); bulkImport.load(); expectLastCall().once(); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/BulkIT.java b/test/src/main/java/org/apache/accumulo/test/functional/BulkIT.java index 131a78f..0c394f2 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/BulkIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/BulkIT.java @@ -117,13 +117,22 @@ public class BulkIT extends AccumuloClusterHarness { c.tableOperations().importDirectory(tableName, files.toString(), bulkFailures.toString(), false); } else { + // not appending the 'ignoreEmptyDir' method defaults to not ignoring empty directories. c.tableOperations().importDirectory(files.toString()).to(tableName).load(); - // rerun using the ignore option and no error should be thrown since empty directories - // will not throw an exception. - c.tableOperations().importDirectory(files.toString(), true).to(tableName).load(); try { - // if run again, without ignore flag, an IllegalArgrumentException should be thrown - c.tableOperations().importDirectory(files.toString(), false).to(tableName).load(); + // if run again, the expected IllegalArgrumentException is thrown + c.tableOperations().importDirectory(files.toString()).to(tableName).load(); + } catch (IllegalArgumentException ex) { + // expected exception to be thrown + } + // re-run using the ignoreEmptyDir option and no error should be thrown since empty + // directories will be ignored + c.tableOperations().importDirectory(files.toString()).to(tableName).ignoreEmptyDir(true) + .load(); + try { + // setting ignoreEmptyDir to false, explicitly, results in exception being thrown again. + c.tableOperations().importDirectory(files.toString()).to(tableName).ignoreEmptyDir(false) + .load(); } catch (IllegalArgumentException ex) { // expected exception to be thrown } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java b/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java index 954d15a..16b9641 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/BulkNewIT.java @@ -151,9 +151,18 @@ public class BulkNewIT extends SharedMiniClusterBase { String h1 = writeData(dir + "/f1.", aconf, 0, 332); c.tableOperations().importDirectory(dir).to(tableName).tableTime(setTime).load(); - // running again with boolean set to true will not throw an exception - c.tableOperations().importDirectory(dir, true).to(tableName).tableTime(setTime).load(); - // but if run with with boolean set to true, an IllegalArgument exception will be thrown + // running again with ignoreEmptyDir set to true will not throw an exception + c.tableOperations().importDirectory(dir).to(tableName).tableTime(setTime).ignoreEmptyDir(true) + .load(); + // but if run with with ignoreEmptyDir value set to false, an IllegalArgument exception will + // be thrown + try { + c.tableOperations().importDirectory(dir).to(tableName).tableTime(setTime) + .ignoreEmptyDir(false).load(); + } catch (IllegalArgumentException ex) { + // expected the exception + } + // or if not supplied at all, the IllegalArgument exception will be thrown as well try { c.tableOperations().importDirectory(dir).to(tableName).tableTime(setTime).load(); } catch (IllegalArgumentException ex) { @@ -480,7 +489,7 @@ public class BulkNewIT extends SharedMiniClusterBase { String dir = getDir("/testBulkFile-"); FileSystem fs = getCluster().getFileSystem(); fs.mkdirs(new Path(dir)); - c.tableOperations().importDirectory(dir, true).to(tableName).load(); + c.tableOperations().importDirectory(dir).to(tableName).ignoreEmptyDir(true).load(); } }