accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Improve property configuration handling of concurrent updates

Open EdColeman opened this issue 2 years ago • 1 comments

Is your feature request related to a problem? Please describe. With recent changes in property handling with version information available we may be able to improve concurrency handling This was pulled from comments on #2799 by @keith-turner

Additional context Directly from @keith-turner comment

[Atomic property update...] this code may be able to gracefully handle multiple clients concurrently modifying props. Not completely sure, but think something like the following may work.

Create a thrift type to return version+props.

class ThriftVersionedProperties {
   long version;
   Map<String, String> props;
}

The following code was written quickly and omits a lot of details, trying to show how we could pass a version back and forth over thrift and use that version to detect concurrent changes and retry.

public void modifyProperties(String tableName, final Consumer<Map<String,String>> mapMutator)
      throws AccumuloException, AccumuloSecurityException, IllegalArgumentException,
      ConcurrentModificationException {

   while(true) {
      // make thrift call to get props+version
      ThriftVersionedProps tvp = client.getTableProperties(...);
      
      // mutate the properties
      mapMutator.accept(tvp.properties);

      try {
          // pass the modified props+version back to thrift call.  The thrift call will throw a thrift exception if the version has changed since we read props above.
          client.modifyTableProperties(...,tableName, tvp);
           // successfully modified props, so break out of the retry loop... probably a better way to do this than a while(true) loop
           break;
      } catch (PropertiesVersionChanged pvc) {
          log.debug("Properties changed since reading, retrying...");
          //TODO maybe sleep using retry object
      } catch (TableNotFoundException e) {
         throw new AccumuloException(e);
      }
   }
}

EdColeman avatar Jul 19 '22 14:07 EdColeman

@EdColeman - this issue can probably be closed as handling concurrent updates are being done in the original issue in #2799

cshannon avatar Sep 09 '22 21:09 cshannon

This was implemented as part of #2967

cshannon avatar Nov 09 '22 18:11 cshannon