[HUDI-7156] Abstract an independent hoodie table filesystem view lock#10197
[HUDI-7156] Abstract an independent hoodie table filesystem view lock#10197zhuanshenbsj1 wants to merge 3 commits intoapache:masterfrom
Conversation
zhuanshenbsj1
force-pushed
the
HUDI-7156
branch
from
cd1abe7 to
0fbcde4
Compare
| * Check if the current thread holds write lock | ||
| */ | ||
| public boolean hasWriteLock() { | ||
| return globalLock.isWriteLockedByCurrentThread(); |
There was a problem hiding this comment.
It seems like the return value is not used, will it block when calling this method? Otherwise we don't need to call it.
There was a problem hiding this comment.
This is a reentrant read-write lock， and with this function we just confirm whether the lock has been acquired. The outer layer has actually acquired the writelock， there we just verify it.
There was a problem hiding this comment.
How do we verify this? I mean, do we need to check the return value of hasWriteLock?
There was a problem hiding this comment.
How do we verify this? I mean, do we need to check the return value of
hasWriteLock?
I'll add a unit test later for this.
There was a problem hiding this comment.
How do we verify this? I mean, do we need to check the return value of
hasWriteLock?
Sorry, I've been busy lately. Added ut to check the value of hasWriteLock, cc~ @stream2000
zhuanshenbsj1
force-pushed
the
HUDI-7156
branch
from
fc156cf to
b68bdcc
Compare
zhuanshenbsj1
force-pushed
the
HUDI-7156
branch
from
b68bdcc to
2cb93bc
Compare
|
|
||
| LOG.info("Timeline Diff Result is :" + diffResult); | ||
|
|
||
| viewLock.hasWriteLock(); |
There was a problem hiding this comment.
Is this line necessary if we're not checking the return value?
There was a problem hiding this comment.
My mistake, assertion has been added
| * Clear the resource. | ||
| */ | ||
| protected void clear() { | ||
| viewLock.hasWriteLock(); |
| * Clear the resource. | ||
| */ | ||
| protected void clear() { | ||
| assert viewLock.hasWriteLock(); |
There was a problem hiding this comment.
We can use ValidationUtils.checkArgument to check the result here.
e.g.,
ValidationUtils.checkArgument(viewLock.hasWriteLock(), "Current thread must hold the write lock on viewLock when clearing the resource");
|
Let's provide more details in the PR description explaining why this PR is necessary. Also, are there plans to support different types of filesystem view locks in the future? |
3 participants
Change Logs
Describe context and summary for this change. Highlight if any code was copied.
Impact
Describe any public API or user-facing feature change or any performance impact.
Risk level (write none, low medium or high below)
If medium or high, explain what verification was done to mitigate the risks.
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist