[GitHub] [accumulo] ctubbsii commented on pull request #2967: Update the new modify properties API to automatically retry on concurrent modification exception.

[GitBox <gi...@apache.org>]

ctubbsii commented on PR #2967:
URL: https://github.com/apache/accumulo/pull/2967#issuecomment-1262680568

   > For the case you describe no one could ever rely on the concurrent modification exception to ensure the correctness of their system. If they want to ensure only a single process modifies configuration this exception does not offer a reliable way to ensure that or detect when that is not the case. I see the exception as a nuisance that may occur for normal use cases in a distributed system, it does not offer anything of value and will just cause people problems IMO.
   
   What I'm suggesting is that internally we check the version and ensure it hasn't changed. If the version has changed because somebody else updated the ZK node with a different edit, then we throw ConcurrentModificationException. That seems like a pretty reliable way to ensure that other changes are detected.
   
   > The wonderful thing about this new API is that allows a user the chance to atomically inspect the config and modify it. This new ability to atomically inspect and modify allows user to build code that is correct in a distributed system on top of this API. If someone wants to utilize this inspect and modify capability I can only think of uses cases where they would want to automatically retry. If their mapMutator inspects before modfifying then it will automatically redo the inspection on the latest snapshot when there is a concurrent modification. Atomic inspect and modify is a really powerful new feature and I think automatic retry makes it easier to use it correctly.
   
   I agree atomic inspect and modify is very powerful, but one of its powers is the ability to detect collisions. I also agree that this empowers correctly written mapMutator code to ensure the desired outcome, even in the face of collisions.
   
   However, my use case that I'm concerned about isn't about the desired outcome of the map mutation. The aspect I'm concerned about is more about administrators / operators being able to detect rogue threads that are causing collisions in a system that an administrator may wish to detect, track down, and address, before they can trust the map mutator code they are trying to run. It's not a "computer science" concern about correctness... it's a practical "IT" concern about system administration. I think you're overlooking that practical concern, even though I agree with you on the computer science/algorithm correctness aspect and the convenience of retrying.
   
   I don't think this will cause people problems if we document it properly in the Javadoc.
   
   All that said, retry may be okay if there's a warning logged (on both the client and the server side, ideally) that notes the change was detected and it will be retried. However, I still have concerns that it may never finish (if we retry indefinitely; very unlikely, and my concern is proportionately low for this one), and the general restriction on users (my concern is higher for this one). Just because we can't think of a use case where a retry is not desired, it doesn't mean there isn't one, and I see no reason to restrict users by adding extra code we have to maintain, just because we failed to have the foresight to think of a way a use case a user might have.


-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org