HDDS-10590. [Snapshot] Synchronized snapshot purge, set snapshot property and SstFilteringService #6456
Conversation
This file contains 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
…y and SstFilteringService
hemantk-12
changed the title
swamirishi
requested changes
Apr 3, 2024
There was a problem hiding this comment.
Thanks for the patch @hemantk-12 , the changes overall look good. I had some nitpicky minor comments.
| // snapshotTableKey is nothing but /volumeName/bucketName/snapshotName. | ||
| // Once all the locks are acquired, it performs the three steps mentioned above and | ||
| // release all the locks after that. | ||
| Set<String> lockSet = new HashSet<>(); |
There was a problem hiding this comment.
Since we know there are going to be only 2 elements in the set. Should we set the initial capacity to 2 and load factor as 1?
Suggested change
| Set<String> lockSet = new HashSet<>(); | |
| Set<String> lockSet = new HashSet<>(2, 1.0); |
There was a problem hiding this comment.
Yes, we can change it to 4 rather than unbounded. We can acquire up to 4 locks (one for the snapshot to purge, one for the next active snapshot to update the deep clean flag, one for the path previous snapshot, and the last one for the global previous).
...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
Outdated
Show resolved
Hide resolved
...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
Outdated
Show resolved
Hide resolved
...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
Outdated
Show resolved
Hide resolved
swamirishi
approved these changes
Apr 3, 2024
...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
Outdated
Show resolved
Hide resolved
...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
Outdated
Show resolved
Hide resolved
|
Thanks @swamirishi for the review. |
xichen01
pushed a commit
to xichen01/ozone
that referenced
this pull request
Apr 17, 2024
…erty and SstFilteringService (apache#6456) (cherry picked from commit 7da5ecb)
xichen01
pushed a commit
to xichen01/ozone
that referenced
this pull request
Apr 18, 2024
…erty and SstFilteringService (apache#6456) (cherry picked from commit 7da5ecb)
xichen01
pushed a commit
to xichen01/ozone
that referenced
this pull request
Apr 23, 2024
…erty and SstFilteringService (apache#6456) (cherry picked from commit 7da5ecb)
jojochuang
pushed a commit
to jojochuang/ozone
that referenced
this pull request
May 29, 2024
…erty and SstFilteringService (apache#6456) (cherry picked from commit 7da5ecb)
swamirishi
pushed a commit
to swamirishi/ozone
that referenced
this pull request
Jun 10, 2024
…napshot property and SstFilteringService (apache#6456) Change-Id: I867aa900c4e76d28abacc2406f6e440769f0b2ce
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changes were proposed in this pull request?
SstFilteringService marks the sstFilter flag for a snapshot and updates the RocksDB, code. It may cause snapshot chain corruption (probably not after the fix: #6443) or data inconsistency due to race condition because SstFilteringService updates snapshot info in parallel to SnapshotPurge and SnapshotProperty which also update snapshot info. Even tho SstFilteringService takes a lock before updating snapshotInfo, SnapshotPurge, and SnapshotProperty APIs don't take a lock and rely on OMStateMachine because OMStateMachine is going to process each request sequentially.
In general, each update to snapshotInfoTable should go through API but that is not possible for SstFilteringService because SstFilteringService runs on each OM independently.
This change introduces locks to achieve synchronization among snapshot purge, set snapshot property, and SstFilteringService.
What is the link to the Apache JIRA
HDDS-10590
How was this patch tested?
Existing unit tests.