[GitHub] [accumulo] keith-turner commented on issue #1358: Fix race conditions with commitsInProgress counter

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

keith-turner commented on issue #1358: Fix race conditions with commitsInProgress counter
URL: https://github.com/apache/accumulo/pull/1358#issuecomment-530859174
 
 
   The code is very complicated for Tablet, but the conceptual model is very simple.  There is a single lock that protects most (there are some exceptions) of a tablets critical in memory data.  A single lock is very easy to reason about.  Multiple locks are hard to reason about. Adding multiple locks piecemeal w/o looking at the bigger picture is not a good idea IMO, I think a complete redesign for Tablet concurrency would be needed.
   
   This code currently expects the Tablet lock to be held when the method is called.  Currently commit sessions are rotated on minor compaction.  The single tablet lock makes this pointer manipulation and access to an individual commit session consistent.  Without the single lock one would need to carefully ensure this consistency is maintained.  If that consistency is not maintained it would lead to many bad things.
   
   Looking into this a bit I think a nice solution for the current code would be to add the following to methods that expect the Tablet lock to be held.  Adding these checks nicely documents the expectation and causes the code to fail at runtime if the expectation is not met.
   
   ```java
     Preconditions.checkState(Thread.holdsLock(tablet);
   ```
   
   For this particular code it would be :
   
   ```java
     Preconditions.checkState(Thread.holdsLock(committer));
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services