Skip to content

Conversation

@hanishakoneru
Copy link
Contributor

What changes were proposed in this pull request?

LevelDB support was removed for OM and SCM DBs but DN Metastore can still be configured to use LevelDB or RocksDB.
This Jira proposes to remove LevelDB configuration option for DN Metastore (ozone.metastore.impl) and use RocksDB only.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-3914

How was this patch tested?

Deprecation patch. No unit tests required.

Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, a few comments inline...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the same reason as above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic needs to be reversed, maybe a typo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will fix it.

@codecov-commenter
Copy link

Codecov Report

Merging #1166 into master will decrease coverage by 0.06%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1166      +/-   ##
============================================
- Coverage     73.49%   73.43%   -0.07%     
+ Complexity    10051    10032      -19     
============================================
  Files           974      974              
  Lines         49731    49729       -2     
  Branches       4893     4893              
============================================
- Hits          36551    36517      -34     
- Misses        10858    10877      +19     
- Partials       2322     2335      +13     
Impacted Files Coverage Δ Complexity Δ
.../java/org/apache/hadoop/ozone/OzoneConfigKeys.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...main/java/org/apache/hadoop/ozone/OzoneConsts.java 85.71% <ø> (ø) 1.00 <0.00> (ø)
...op/ozone/container/keyvalue/KeyValueContainer.java 75.00% <ø> (-0.18%) 51.00 <0.00> (ø)
...one/container/keyvalue/KeyValueContainerCheck.java 75.36% <0.00%> (-0.73%) 21.00 <0.00> (-1.00)
...ava/org/apache/hadoop/hdds/utils/LevelDBStore.java 72.34% <ø> (-1.42%) 38.00 <0.00> (-2.00)
...apache/hadoop/hdds/utils/LevelDBStoreIterator.java 46.15% <ø> (-53.85%) 3.00 <0.00> (-3.00)
...apache/hadoop/hdds/utils/MetadataStoreBuilder.java 87.23% <60.00%> (-0.27%) 14.00 <0.00> (ø)
...zone/container/keyvalue/KeyValueContainerData.java 69.35% <100.00%> (+0.50%) 17.00 <0.00> (ø)
...hdds/scm/container/common/helpers/ExcludeList.java 86.95% <0.00%> (-13.05%) 19.00% <0.00%> (-3.00%)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d85c7e3...6ff2338. Read the comment docs.

Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hanishakoneru for the update. LGTM, +1.

@vivekratnavel
Copy link
Contributor

@hanishakoneru Thanks for working on this and @xiaoyuyao thanks for the review!

@vivekratnavel vivekratnavel merged commit 2af6198 into apache:master Jul 10, 2020
ChenSammi pushed a commit that referenced this pull request Jul 22, 2020
rakeshadr pushed a commit to rakeshadr/hadoop-ozone that referenced this pull request Sep 3, 2020
@hanishakoneru hanishakoneru deleted the HDDS-3914 branch December 1, 2020 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants