This is an automated email from the ASF dual-hosted git repository. mmiller 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 e4299fa5b8 Add new CloneConfiguration API method (#2874) e4299fa5b8 is described below commit e4299fa5b82f7fcd835b5e81669882d90aa99ebd Author: Mike Miller <mmil...@apache.org> AuthorDate: Fri Aug 12 12:43:31 2022 +0000 Add new CloneConfiguration API method (#2874) * Add new convenience method for empty object * Create private methods for reuse in TableOperationsImpl * Some various code clean up --- .../core/client/admin/CloneConfiguration.java | 30 +++++++++------ .../core/clientImpl/CloneConfigurationImpl.java | 7 ++-- .../core/clientImpl/TableOperationsImpl.java | 43 +++++++++++++--------- .../accumulo/manager/FateServiceHandler.java | 6 +-- .../accumulo/test/functional/CloneTestIT.java | 8 ++-- 5 files changed, 55 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java index fa2c104258..a47fa7a58a 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/client/admin/CloneConfiguration.java @@ -35,14 +35,14 @@ public interface CloneConfiguration { * * @return true if memory is flushed in the source table before cloning. */ - public boolean isFlush(); + boolean isFlush(); /** * The source table properties are copied. This allows overriding of some of those properties. * * @return The source table properties to override. */ - public Map<String,String> getPropertiesToSet(); + Map<String,String> getPropertiesToSet(); /** * The source table properties are copied, this allows reverting to system defaults for some of @@ -50,7 +50,7 @@ public interface CloneConfiguration { * * @return The properties that are to be reverted to system defaults. */ - public Set<String> getPropertiesToExclude(); + Set<String> getPropertiesToExclude(); /** * The new table is normally brought online after the cloning process. This allows leaving the new @@ -58,21 +58,21 @@ public interface CloneConfiguration { * * @return true if the new table is to be kept offline after cloning. */ - public boolean isKeepOffline(); + boolean isKeepOffline(); /** * A CloneConfiguration builder * * @since 1.10 and 2.1 */ - public static interface Builder { + interface Builder { /** * Determines if memory is flushed in the source table before cloning. * * @param flush * true if memory is flushed in the source table before cloning. */ - public Builder setFlush(boolean flush); + Builder setFlush(boolean flush); /** * The source table properties are copied. This allows overriding of some of those properties. @@ -80,7 +80,7 @@ public interface CloneConfiguration { * @param propertiesToSet * The source table properties to override. */ - public Builder setPropertiesToSet(Map<String,String> propertiesToSet); + Builder setPropertiesToSet(Map<String,String> propertiesToSet); /** * The source table properties are copied, this allows reverting to system defaults for some of @@ -89,7 +89,7 @@ public interface CloneConfiguration { * @param propertiesToExclude * The properties that are to be reverted to system defaults. */ - public Builder setPropertiesToExclude(Set<String> propertiesToExclude); + Builder setPropertiesToExclude(Set<String> propertiesToExclude); /** * The new table is normally brought online after the cloning process. This allows leaving the @@ -98,21 +98,29 @@ public interface CloneConfiguration { * @param keepOffline * true if the new table is to be kept offline after cloning. */ - public Builder setKeepOffline(boolean keepOffline); + Builder setKeepOffline(boolean keepOffline); /** * Build the clone configuration * * @return the built immutable clone configuration */ - public CloneConfiguration build(); + CloneConfiguration build(); } /** * @return a {@link CloneConfiguration} builder */ - public static CloneConfiguration.Builder builder() { + static CloneConfiguration.Builder builder() { return new CloneConfigurationImpl(); } + /** + * @return an empty configuration object with the default settings. + * @since 2.1.0 + */ + static CloneConfiguration empty() { + return builder().build(); + } + } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java index d380090dae..9549fd6c5d 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/CloneConfigurationImpl.java @@ -33,8 +33,7 @@ import com.google.common.base.Preconditions; */ public class CloneConfigurationImpl implements CloneConfiguration, CloneConfiguration.Builder { - // The purpose of this is to allow building an immutable CloneConfiguration object without - // creating + // This boolean allows building an immutable CloneConfiguration object without creating // separate Builder and CloneConfiguration objects. This is done to reduce object creation and // copying. This could easily be changed to two objects without changing the interfaces. private boolean built = false; @@ -62,14 +61,14 @@ public class CloneConfigurationImpl implements CloneConfiguration, CloneConfigur @Override public Map<String,String> getPropertiesToSet() { Preconditions.checkState(built); - return (propertiesToSet == null ? Collections.<String,String>emptyMap() + return (propertiesToSet == null ? Collections.emptyMap() : Collections.unmodifiableMap(propertiesToSet)); } @Override public Set<String> getPropertiesToExclude() { Preconditions.checkState(built); - return (propertiesToExclude == null ? Collections.<String>emptySet() + return (propertiesToExclude == null ? Collections.emptySet() : Collections.unmodifiableSet(propertiesToExclude)); } 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 60868ba229..e2cfc4840c 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 @@ -156,7 +156,7 @@ public class TableOperationsImpl extends TableOperationsHelper { private static final SecureRandom random = new SecureRandom(); - public static final String CLONE_EXCLUDE_PREFIX = "!"; + public static final String PROPERTY_EXCLUDE_PREFIX = "!"; public static final String COMPACTION_CANCELED_MSG = "Compaction canceled"; public static final String TABLE_DELETED_MSG = "Table is being deleted"; @@ -772,33 +772,21 @@ public class TableOperationsImpl extends TableOperationsHelper { throws AccumuloSecurityException, TableNotFoundException, AccumuloException, TableExistsException { NEW_TABLE_NAME.validate(newTableName); + requireNonNull(config, "CloneConfiguration required."); TableId srcTableId = context.getTableId(srcTableName); if (config.isFlush()) _flush(srcTableId, null, null, true); - Set<String> propertiesToExclude = config.getPropertiesToExclude(); - if (propertiesToExclude == null) - propertiesToExclude = Collections.emptySet(); - - Map<String,String> propertiesToSet = config.getPropertiesToSet(); - if (propertiesToSet == null) - propertiesToSet = Collections.emptyMap(); + Map<String,String> opts = new HashMap<>(); + validatePropertiesToSet(opts, config.getPropertiesToSet()); List<ByteBuffer> args = Arrays.asList(ByteBuffer.wrap(srcTableId.canonical().getBytes(UTF_8)), ByteBuffer.wrap(newTableName.getBytes(UTF_8)), ByteBuffer.wrap(Boolean.toString(config.isKeepOffline()).getBytes(UTF_8))); - Map<String,String> opts = new HashMap<>(); - for (Entry<String,String> entry : propertiesToSet.entrySet()) { - if (entry.getKey().startsWith(CLONE_EXCLUDE_PREFIX)) - throw new IllegalArgumentException("Property can not start with " + CLONE_EXCLUDE_PREFIX); - opts.put(entry.getKey(), entry.getValue()); - } - for (String prop : propertiesToExclude) { - opts.put(CLONE_EXCLUDE_PREFIX + prop, ""); - } + prependPropertiesToExclude(opts, config.getPropertiesToExclude()); doTableFateOperation(newTableName, AccumuloException.class, FateOperation.TABLE_CLONE, args, opts); @@ -2017,4 +2005,25 @@ public class TableOperationsImpl extends TableOperationsHelper { public ImportDestinationArguments importDirectory(String directory) { return new BulkImport(directory, context); } + + private void prependPropertiesToExclude(Map<String,String> opts, Set<String> propsToExclude) { + if (propsToExclude == null) + return; + + for (String prop : propsToExclude) { + opts.put(PROPERTY_EXCLUDE_PREFIX + prop, ""); + } + } + + private void validatePropertiesToSet(Map<String,String> opts, Map<String,String> propsToSet) { + if (propsToSet == null) + return; + + propsToSet.forEach((k, v) -> { + if (k.startsWith(PROPERTY_EXCLUDE_PREFIX)) + throw new IllegalArgumentException( + "Property can not start with " + PROPERTY_EXCLUDE_PREFIX); + opts.put(k, v); + }); + } } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index efac03777a..41ce18c9ff 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -301,9 +301,9 @@ class FateServiceHandler implements FateService.Iface { Set<String> propertiesToExclude = new HashSet<>(); for (Entry<String,String> entry : options.entrySet()) { - if (entry.getKey().startsWith(TableOperationsImpl.CLONE_EXCLUDE_PREFIX)) { - propertiesToExclude - .add(entry.getKey().substring(TableOperationsImpl.CLONE_EXCLUDE_PREFIX.length())); + if (entry.getKey().startsWith(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX)) { + propertiesToExclude.add( + entry.getKey().substring(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX.length())); continue; } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java b/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java index e7bd4a2aa1..a533ec3f9a 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/CloneTestIT.java @@ -318,7 +318,7 @@ public class CloneTestIT extends AccumuloClusterHarness { bw.addMutations(mutations); } - client.tableOperations().clone(tables[0], tables[1], true, null, null); + client.tableOperations().clone(tables[0], tables[1], CloneConfiguration.empty()); client.tableOperations().deleteRows(tables[1], new Text("4"), new Text("8")); @@ -338,15 +338,15 @@ public class CloneTestIT extends AccumuloClusterHarness { public void testCloneRootTable() { try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { assertThrows(AccumuloException.class, - () -> client.tableOperations().clone(RootTable.NAME, "rc1", true, null, null)); + () -> client.tableOperations().clone(RootTable.NAME, "rc1", CloneConfiguration.empty())); } } @Test public void testCloneMetadataTable() { try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { - assertThrows(AccumuloException.class, - () -> client.tableOperations().clone(MetadataTable.NAME, "mc1", true, null, null)); + assertThrows(AccumuloException.class, () -> client.tableOperations().clone(MetadataTable.NAME, + "mc1", CloneConfiguration.empty())); } } }