adds ranges to table locks #5786
Merged
+997
−199
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added ranges to table locks using the ranges from fate operations. For write locks these ranges are widened to the nearest splits, see comments in Utils.reserveTable for details. See comments and code in DistributedReadWriteLock.WriteLock.tryLock() and DistributedReadWriteLock.ReadLock.tryLock() for details about how these ranges change lock behavior. Added tests to excercise this new locking behavior. Modified the `accumulo admin fate -l` command to display lock ranges. Also modified how this command determines if a lock is held or waiting. fixes apache#2483
Contributor
Author
|
This change is important for the new automatic merge feature. Even though merges are fast now, if a merge gets stuck waiting on a long running compaction then it will block all subsequent compactions and bulk imports from running. Adding ranges prevents blocking for the case where the ranges do not overlap. |
kevinrr888
reviewed
Aug 14, 2025
Member
kevinrr888
left a comment
kevinrr888
left a comment
There was a problem hiding this comment.
I haven't looked through everything, posting comments I have. Overall, this looks like a very nice improvement to table locking.
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/LockRange.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/apache/accumulo/core/fate/zookeeper/DistributedReadWriteLockTest.java
Show resolved
Hide resolved
...er/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/ComputeBulkRange.java
Show resolved
Hide resolved
…eLock.java Co-authored-by: Kevin Rathbun <kevinrr888@gmail.com>
…eLock.java Co-authored-by: Kevin Rathbun <kevinrr888@gmail.com>
…kRange.java Co-authored-by: Kevin Rathbun <kevinrr888@gmail.com>
kevinrr888
approved these changes
Aug 27, 2025
Member
kevinrr888
left a comment
kevinrr888
left a comment
There was a problem hiding this comment.
Looks good. I didn't follow all of the bulk import changes, but looks like you added more bulk import testing for these changes as well. May be good to run full set of ITs before this is merged
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
Show resolved
Hide resolved
Comment on lines
+156
to
+177
| /* | ||
| * Write locks are widened to table split points to avoid non-overlapping ranges operating on | ||
| * the same tablet. For example assume a table has splits at c,e and two fate operations need | ||
| * write locks on ranges (a1,c2] and (d1,d9]. The fate ranges do not overlap, however both | ||
| * will operate on the tablet (c,e]. When the ranges are widened, (a1,c2] turns into (null,e] | ||
| * and (d1,d9] turns into (c,e]. The widened ranges for the fate operations overlap which | ||
| * prevents both from operating on the same tablet. | ||
| * | ||
| * Widening is done for write locks because those need to work on mutually exclusive sets of | ||
| * tablets w/ other write or read locks. Read locks do not need to be mutually exclusive w/ | ||
| * each other and it does not matter if they operate on the same tablet so widening is not | ||
| * needed for that case. Widening write locks is sufficient for detecting overlap w/ any | ||
| * tablets that read locks need to use. For example if a write locks is obtained on (a1,c2] | ||
| * and it widens to (null,e] then a read lock on (d1,d9] would overlap with the widened range. | ||
| * | ||
| * Mostly widening for write locks is nice because fate operations that get read locks are | ||
| * probably more frequent. So the work of widening is not done on most fate operations. Also | ||
| * widening an infinite range is a quick operation, so create/delete table will not be slowed | ||
| * down by widening. | ||
| * | ||
| * Widening is done for compactions because those operations widen their range. | ||
| */ |
Member
There was a problem hiding this comment.
Very good explanation. I wonder if there's a better/more visible spot for this. This spot seems fine though
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/Utils.java
Outdated
Show resolved
Hide resolved
DomGarguilo
reviewed
Aug 27, 2025
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/FateLock.java
Show resolved
Hide resolved
Contributor
Author
|
I was running bulk random walk test against this branch and found #5833. The test is a good test for this change because it does lots of concurrent table operations with different ranges. |
…Ops/Utils.java Co-authored-by: Kevin Rathbun <kevinrr888@gmail.com>
Contributor
Author
The cause of the bulk changes was the code was computing the range after it got the lock. Needed to reorg the code to compute the range prior to getting the lock. |
keith-turner
added a commit
to keith-turner/accumulo
that referenced
this pull request
Aug 28, 2025
This was referenced Aug 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added ranges to table locks using the ranges from fate operations. For write locks these ranges are widened to the nearest splits, see comments in Utils.reserveTable for details. See comments and code in DistributedReadWriteLock.WriteLock.tryLock() and
DistributedReadWriteLock.ReadLock.tryLock() for details about how these ranges change lock behavior. Added tests to excercise this new locking behavior. Modified the
accumulo admin fate -lcommand to display lock ranges. Also modified how this command determines if a lock is held or waiting.fixes #2483