keith-turner commented on code in PR #2965:
URL: https://github.com/apache/accumulo/pull/2965#discussion_r981216116
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java:
##########
@@ -56,10 +57,12 @@ public void attachIterator(String tableName,
IteratorSetting setting,
for (IteratorScope scope : scopes) {
String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX,
scope.name().toLowerCase(), setting.getName());
- for (Entry<String,String> prop : setting.getOptions().entrySet()) {
- this.setProperty(tableName, root + ".opt." + prop.getKey(),
prop.getValue());
- }
- this.setProperty(tableName, root, setting.getPriority() + "," +
setting.getIteratorClass());
+
+ Map<String,String> propsToAdd = setting.getOptions().entrySet().stream()
+ .collect(Collectors.toMap(prop -> root + ".opt." + prop.getKey(),
Entry::getValue));
+ propsToAdd.put(root, setting.getPriority() + "," +
setting.getIteratorClass());
+
+ this.modifyProperties(tableName, props -> props.putAll(propsToAdd));
Review Comment:
> Another option could be to catch and turn the
ConcurrentModificationException into a CheckedException so that callers are
forced to handle it.
I am not a fan of this personally. Looking at this PR, each call would have
to handle the same case. So that would lead to creating an internal static
utility method that does the retry.
> If the retry allows different, non-overlapping changes, that might be the
"best" case. P1 sets A, B, and P2 sets C, D then the final result should be A,
B, C and D. The "worst" case would be if the final values were A,B or C,D
>
>If the sets are overlapping then the result would be non-determinant P1
sets A, deletes B and P2 sets B and C, then B could be set or deleted. Changing
the same property should be much less frequent than modifying non-overlapping
sets.
I created #2967 to do the retry and in that PR I also added a rigorous
concurrency test. The test makes lots of modifications in many threads and can
detect if any single modification is lost. The test also has some thread
modify the same properties.
Ideally when multiple threads are concurrently modifying properties, that
should result in the same outcome as if all those modifications were done
serially in some order. Barring bugs, conceptually I think the current
accumulo code should be able to achieve this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
