-
Notifications
You must be signed in to change notification settings - Fork 587
HDDS-3479. Use SCMMetadataStore high level abstraction instead of DBS… #997
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
Conversation
…toreBuilder and SCMDBDefinition directly HDDS-3479. Rename SCMMetadataStoreRDBImpl to SCMMetadataStoreImpl as it is not longer RocksDB specific.
|
The acceptance check has some errors, specifically, test cases related to S3 in this part Run cd /mnt/ozone/compose && ./test-all.sh. I don't think these errors are caused by the changes introduced in this PR. |
elek
left a comment
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.
+1
Thanks the patch @jsoft88 (And sorry for the late review).
One NIT: There are two lines which are commented out: I would prefer to remove them
Pushed the removal and a rebase to your fork (to make the merge faster). Will merge it after a green build.)
elek
left a comment
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.
+1 LGTM
Merging it to the master Thanks the contribution @jsoft88
…toreBuilder and SCMDBDefinition directly
HDDS-3479. Rename SCMMetadataStoreRDBImpl to SCMMetadataStoreImpl as it is not longer RocksDB specific.
What changes were proposed in this pull request?
Use ScmMetadataStore everywhere inside SCM instead of DBStoreBuilder with separated DB definition (SCMDBDefinition)
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3479?jql=project%20in%20(HDDS)%20AND%20labels%20in%20(newbie)%20AND%20assignee%20is%20EMPTY%20AND%20status%20in%20(open%2C%20Reopened)
How was this patch tested?
By executing test.sh under dist/