This is an automated email from the ASF dual-hosted git repository. domgarguilo 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 f79cea1610 Make several TableOpsImpl methods atomic (#5540) f79cea1610 is described below commit f79cea16103e505ced6d8a4d85affcdfc33aa1c6 Author: Dom G. <domgargu...@apache.org> AuthorDate: Wed Jul 2 12:45:47 2025 -0400 Make several TableOpsImpl methods atomic (#5540) * Make setLocalityGroups() atomic * make samplerConfig methods atomic * simplfy using removeIf() * Unwrap TNFE for changed methods * Create new TNFE as to not lose stack info when unwrapping * Apply suggestions from code review Co-authored-by: Christopher Tubbs <ctubb...@apache.org> * Formatting --------- Co-authored-by: Christopher Tubbs <ctubb...@apache.org> --- .../core/clientImpl/TableOperationsImpl.java | 96 +++++++++++----------- 1 file changed, 50 insertions(+), 46 deletions(-) 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 0fd16d8374..ba8bba53f1 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 @@ -1109,6 +1109,26 @@ public class TableOperationsImpl extends TableOperationsHelper { } } + /** + * Like modifyProperties(...), but if an AccumuloException is caused by a TableNotFoundException, + * unwrap and rethrow the TNFE directly. This is a hacky, temporary workaround that we can use + * until we are able to change the public API and throw TNFE directly from all applicable methods. + */ + private Map<String,String> modifyPropertiesUnwrapped(String tableName, + Consumer<Map<String,String>> mapMutator) + throws TableNotFoundException, AccumuloException, AccumuloSecurityException { + + try { + return modifyProperties(tableName, mapMutator); + } catch (AccumuloException ae) { + Throwable cause = ae.getCause(); + if (cause instanceof TableNotFoundException) { + throw new TableNotFoundException(null, tableName, null, ae); + } + throw ae; + } + } + private void setPropertyNoChecks(final String tableName, final String property, final String value) throws AccumuloException, AccumuloSecurityException, TableNotFoundException { @@ -1206,42 +1226,32 @@ public class TableOperationsImpl extends TableOperationsHelper { } @Override - public void setLocalityGroups(String tableName, Map<String,Set<Text>> groups) + public void setLocalityGroups(String tableName, Map<String,Set<Text>> groupsToSet) throws AccumuloException, AccumuloSecurityException, TableNotFoundException { // ensure locality groups do not overlap - LocalityGroupUtil.ensureNonOverlappingGroups(groups); + LocalityGroupUtil.ensureNonOverlappingGroups(groupsToSet); - for (Entry<String,Set<Text>> entry : groups.entrySet()) { - Set<Text> colFams = entry.getValue(); - String value = LocalityGroupUtil.encodeColumnFamilies(colFams); - setPropertyNoChecks(tableName, Property.TABLE_LOCALITY_GROUP_PREFIX + entry.getKey(), value); - } + final String localityGroupPrefix = Property.TABLE_LOCALITY_GROUP_PREFIX.getKey(); - try { - setPropertyNoChecks(tableName, Property.TABLE_LOCALITY_GROUPS.getKey(), - Joiner.on(",").join(groups.keySet())); - } catch (AccumuloException e) { - if (e.getCause() instanceof TableNotFoundException) { - throw (TableNotFoundException) e.getCause(); - } - throw e; - } + modifyPropertiesUnwrapped(tableName, properties -> { - // remove anything extraneous - String prefix = Property.TABLE_LOCALITY_GROUP_PREFIX.getKey(); - for (Entry<String,String> entry : getProperties(tableName)) { - String property = entry.getKey(); - if (property.startsWith(prefix)) { - // this property configures a locality group, find out which - // one: - String[] parts = property.split("\\."); - String group = parts[parts.length - 1]; + // add/update each locality group + groupsToSet.forEach((groupName, colFams) -> properties.put(localityGroupPrefix + groupName, + LocalityGroupUtil.encodeColumnFamilies(colFams))); - if (!groups.containsKey(group)) { - removePropertyNoChecks(tableName, property); + // update the list of all locality groups + final String allGroups = Joiner.on(",").join(groupsToSet.keySet()); + properties.put(Property.TABLE_LOCALITY_GROUPS.getKey(), allGroups); + + // remove any stale locality groups that were previously set + properties.keySet().removeIf(property -> { + if (property.startsWith(localityGroupPrefix)) { + String group = property.substring(localityGroupPrefix.length()); + return !groupsToSet.containsKey(group); } - } - } + return false; + }); + }); } @Override @@ -1846,28 +1856,19 @@ public class TableOperationsImpl extends TableOperationsHelper { } } - private void clearSamplerOptions(String tableName) - throws AccumuloException, TableNotFoundException, AccumuloSecurityException { - EXISTING_TABLE_NAME.validate(tableName); - - String prefix = Property.TABLE_SAMPLER_OPTS.getKey(); - for (Entry<String,String> entry : getProperties(tableName)) { - String property = entry.getKey(); - if (property.startsWith(prefix)) { - removeProperty(tableName, property); - } - } - } - @Override public void setSamplerConfiguration(String tableName, SamplerConfiguration samplerConfiguration) throws AccumuloException, TableNotFoundException, AccumuloSecurityException { EXISTING_TABLE_NAME.validate(tableName); - clearSamplerOptions(tableName); Map<String,String> props = new SamplerConfigurationImpl(samplerConfiguration).toTablePropertiesMap(); - modifyProperties(tableName, properties -> properties.putAll(props)); + + modifyPropertiesUnwrapped(tableName, properties -> { + properties.keySet() + .removeIf(property -> property.startsWith(Property.TABLE_SAMPLER_OPTS.getKey())); + properties.putAll(props); + }); } @Override @@ -1875,8 +1876,11 @@ public class TableOperationsImpl extends TableOperationsHelper { throws AccumuloException, TableNotFoundException, AccumuloSecurityException { EXISTING_TABLE_NAME.validate(tableName); - removeProperty(tableName, Property.TABLE_SAMPLER.getKey()); - clearSamplerOptions(tableName); + modifyPropertiesUnwrapped(tableName, properties -> { + properties.remove(Property.TABLE_SAMPLER.getKey()); + properties.keySet() + .removeIf(property -> property.startsWith(Property.TABLE_SAMPLER_OPTS.getKey())); + }); } @Override