[#778] core(feat): Support Tree lock #1264
Conversation
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
|
@jerryshao |
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
|
@jerryshao
Additionally, I will add a new issue to reuse tree locks to reduce memory pressure. |
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/lock/TreeLockConfigs.java
Outdated
Show resolved
Hide resolved
|
|
||
| // Handle self node. | ||
| if (treeNode.getReference() == 0) { | ||
| synchronized (parent) { |
There was a problem hiding this comment.
The logic for cleaning nodes should be placed in TreeLockNode. TreeLockManager should only be responsible for selecting the TreeLockNode nodes that need to be cleaned, that is, those with a refcount of 0.
There was a problem hiding this comment.
We need to verify the child reference count in the synchronization block, it's not feasible to use a synchronized method here.
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/lock/LockManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/lock/TreeLockNode.java
Outdated
Show resolved
Hide resolved
| /** Unlock the tree lock. */ | ||
| public void unlock() { | ||
| if (lockType == null) { | ||
| throw new IllegalStateException("We must lock the tree lock before unlock it."); |
There was a problem hiding this comment.
We need throw Exception on unlock function. If the user code throw exception when get TreeLock but not lock. the finally code block should call unlock.
There was a problem hiding this comment.
This is just defensive programming. We can make sure that users will not call it before lock
core/src/test/java/com/datastrato/gravitino/lock/TestLockManager.java
Outdated
Show resolved
Hide resolved
|
@jerryshao |
|
@diqiu50 you can go ahead if you think fine. |
|
Thanks all for the contribution. I'm going to merge this. |
What changes were proposed in this pull request?
Add the tree-like locks to enhance performance.
Why are the changes needed?
Add the tree-like locks to enhance performance.
Fix: #778
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
UT,
TestLockManager