Skip to content

Commit 8d7c544

Browse files
authored
fails on iterator configuration conflicts (#6122)
This is a follow on issue to #6040 that makes all code fail when detecting iterator conflicts instead of logging at warn.
1 parent 416ba05 commit 8d7c544

10 files changed

Lines changed: 205 additions & 237 deletions

File tree

core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ public void checkIteratorConflicts(String namespace, IteratorSetting setting,
148148
throw new NamespaceNotFoundException(null, namespace, null);
149149
}
150150
var props = this.getNamespaceProperties(namespace);
151-
IteratorConfigUtil.checkIteratorConflicts("namespace:" + namespace, props, setting, scopes,
152-
true);
151+
IteratorConfigUtil.checkIteratorConflicts("namespace:" + namespace, props, setting, scopes);
153152
}
154153

155154
@Override

core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public static void checkIteratorConflicts(Map<String,String> props, IteratorSett
140140
EnumSet<IteratorScope> scopes) throws AccumuloException {
141141
checkArgument(setting != null, "setting is null");
142142
checkArgument(scopes != null, "scopes is null");
143-
IteratorConfigUtil.checkIteratorConflicts("", props, setting, scopes, true);
143+
IteratorConfigUtil.checkIteratorConflicts("", props, setting, scopes);
144144
}
145145

146146
@Override

core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,9 @@ public static void checkIteratorPriorityConflicts(String errorContext,
288288
// properties.
289289
if (iterProps.stream()
290290
.anyMatch(iterProp -> newProperties.containsKey(iterProp.getProperty()))) {
291-
log.warn("For {}, newly set property introduced an iterator priority conflict : {}",
292-
errorContext, iterProps);
291+
throw new IllegalArgumentException(String.format(
292+
"For %s, newly set property introduced an iterator priority conflict : %s",
293+
errorContext, iterProps));
293294
}
294295
}
295296
});
@@ -298,8 +299,7 @@ public static void checkIteratorPriorityConflicts(String errorContext,
298299

299300
public static void checkIteratorConflicts(String logContext, IteratorSetting iterToCheck,
300301
EnumSet<IteratorScope> iterScopesToCheck,
301-
Map<IteratorScope,List<IteratorSetting>> existingIters, boolean shouldThrow)
302-
throws AccumuloException {
302+
Map<IteratorScope,List<IteratorSetting>> existingIters) throws AccumuloException {
303303
// The reason for the 'shouldThrow' var is to prevent newly added 2.x checks from breaking
304304
// existing user code. Just log the problem and proceed. Major version > 2 will always throw
305305
for (var scope : iterScopesToCheck) {
@@ -316,28 +316,20 @@ public static void checkIteratorConflicts(String logContext, IteratorSetting ite
316316
String msg =
317317
String.format("%s iterator name conflict at %s scope. %s conflicts with existing %s",
318318
logContext, scope, iterToCheck, existingIter);
319-
if (shouldThrow) {
320-
throw new AccumuloException(new IllegalArgumentException(msg));
321-
} else {
322-
log.warn(msg + WARNING_MSG);
323-
}
319+
throw new AccumuloException(new IllegalArgumentException(msg));
324320
}
325321
if (iterToCheck.getPriority() == existingIter.getPriority()) {
326322
String msg = String.format(
327323
"%s iterator priority conflict at %s scope. %s conflicts with existing %s",
328324
logContext, scope, iterToCheck, existingIter);
329-
if (shouldThrow) {
330-
throw new AccumuloException(new IllegalArgumentException(msg));
331-
} else {
332-
log.warn(msg + WARNING_MSG);
333-
}
325+
throw new AccumuloException(new IllegalArgumentException(msg));
334326
}
335327
}
336328
}
337329
}
338330

339331
public static void checkIteratorConflicts(String logContext, Map<String,String> props,
340-
IteratorSetting iterToCheck, EnumSet<IteratorScope> iterScopesToCheck, boolean shouldThrow)
332+
IteratorSetting iterToCheck, EnumSet<IteratorScope> iterScopesToCheck)
341333
throws AccumuloException {
342334
// parse the props map
343335
Map<IteratorScope,Map<String,IteratorSetting>> iteratorSettings = new HashMap<>();
@@ -368,11 +360,7 @@ public static void checkIteratorConflicts(String logContext, Map<String,String>
368360
String msg = String.format(
369361
"%s iterator name conflict at %s scope. %s conflicts with existing %s", logContext,
370362
iterProp.getScope(), iterToCheck, iterProp);
371-
if (shouldThrow) {
372-
throw new AccumuloException(new IllegalArgumentException(msg));
373-
} else {
374-
log.warn(msg + WARNING_MSG);
375-
}
363+
throw new AccumuloException(new IllegalArgumentException(msg));
376364
}
377365
} else {
378366
iterSetting.addOption(iterProp.getOptionKey(), iterProp.getOptionValue());
@@ -381,6 +369,6 @@ public static void checkIteratorConflicts(String logContext, Map<String,String>
381369
}
382370

383371
// check if the given iterator conflicts with any existing iterators
384-
checkIteratorConflicts(logContext, iterToCheck, iterScopesToCheck, existingIters, shouldThrow);
372+
checkIteratorConflicts(logContext, iterToCheck, iterScopesToCheck, existingIters);
385373
}
386374
}

0 commit comments

Comments
 (0)