HDDS-13856. Change SstFileInfo to track fileName as the name of the file without sst extension#9221
Conversation
…ile without sst extension Change-Id: Iec643f1cb38be6ee1f58ce67e97c3021bf8ab368
There was a problem hiding this comment.
@swamirishi Thanks for working on this,
Could you please make the required changes to testCreateNewSnapshotLocalYaml as it would lead to test failure because the test still expects filenames with .sst, causing mismatch.
There was a problem hiding this comment.
Done moved the test to TestOmSnapshotLocalDataManager since the code has been refactored in the jira in HDDS-13783
Change-Id: Ie56b389d802fa43cf1564849aab5bbcef7c79438
...-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/CompactionNode.java
Show resolved
Hide resolved
jojochuang
left a comment
jojochuang
left a comment
There was a problem hiding this comment.
i think this is pretty much ready to merge. Let's address the other review comment and then it can merge.
|
|
||
| @VisibleForTesting | ||
| static boolean shouldSkipNode(CompactionNode node, | ||
| static boolean shouldSkipNode(SstFileInfo node, |
There was a problem hiding this comment.
seems unnecessary change.
There was a problem hiding this comment.
We would need this change later
sreejasahithi
left a comment
sreejasahithi
left a comment
There was a problem hiding this comment.
Thanks @swamirishi , overall LGTM , please address the remaining comments.
Change-Id: If049d818575d3a3b03eed707ba240827d512a015
Change-Id: I9db2d1d9430cf11209c3e3a36da3ac11e9a1cb35
|
Thank you @sreejasahithi and @jojochuang for reviewing the patch |
|
@swamirishi please set fix version when resolving Jira issue after PR merge |
4 participants
What changes were proposed in this pull request?
Change SstFileInfo to track fileName as the name of the file without sst extension similar to CompactionFileInfo and CompactionNode class. Refactor CompactionNode to extend SstFileInfo since it shares all the common fields.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13856
How was this patch tested?
Added Unit tests