-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#643] improvement(core): Improve and fix the possible concurrent problem when visiting EntityStore #654
Conversation
Code Coverage Report
Files
|
core/src/main/java/com/datastrato/gravitino/storage/kv/KvEntityStore.java
Outdated
Show resolved
Hide resolved
Assertions.assertEquals(9, totalFailed); | ||
|
||
} finally { | ||
FileUtils.deleteDirectory(FileUtils.getFile("/tmp/testConcurrentIssues")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use the TempDir
of Junit
here? We don't need care the deletion of the file if we use TempDir
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.
The current change LGTM. @qqqttt123 do you have any more comment? |
What changes were proposed in this pull request?
Introducing the
reentrantReadWriteLock
to alleviate possible concurrent problems to access theKvEntityStore
Why are the changes needed?
KvEntityStore
subjects to concurrent problems when multi-thread access it at the same time. For instance, if there aremultiple threads are attempting to save the entity with the same name identifier into the store. We must ensure that the entity can only be saved successfully once.
Fix: #643
Fix: #618
Does this PR introduce any user-facing change?
No
How was this patch tested?
New test case named
testConcurrentIssues
inTestKvEntityStorage
added.