You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/07/06 20:09:03 UTC

[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

hanishakoneru opened a new pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166


   ## 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.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] vivekratnavel merged pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
vivekratnavel merged pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#discussion_r452392038



##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestMetadataStore.java
##########
@@ -90,12 +93,12 @@ public void init() throws IOException {
         + "-" + storeImpl.toLowerCase());
 
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.set(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL, storeImpl);
 
     store = MetadataStoreBuilder.newBuilder()
         .setConf(conf)
         .setCreateIfMissing(true)
         .setDbFile(testDir)
+        .setDBType(storeImpl)

Review comment:
       For the same reason as above.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#discussion_r452392147



##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestMetadataStore.java
##########
@@ -166,12 +170,12 @@ public void testIterator() throws Exception {
   public void testMetaStoreConfigDifferentFromType() throws IOException {
 
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.set(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL, storeImpl);
+
     String dbType;
     GenericTestUtils.setLogLevel(MetadataStoreBuilder.LOG, Level.DEBUG);
     GenericTestUtils.LogCapturer logCapturer =
         GenericTestUtils.LogCapturer.captureLogs(MetadataStoreBuilder.LOG);
-    if (storeImpl.equals(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL_LEVELDB)) {
+    if (storeImpl.equals(CONTAINER_DB_TYPE_LEVELDB)) {

Review comment:
       Thanks. Will fix it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#issuecomment-656347397


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=h1) Report
   > Merging [#1166](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/d85c7e334c568a9df571b7b779d119a0a1566751&el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=tree)
   
   ```diff
   @@             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](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/hadoop/ozone/OzoneConfigKeys.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvT3pvbmVDb25maWdLZXlzLmphdmE=) | `100.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...main/java/org/apache/hadoop/ozone/OzoneConsts.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvT3pvbmVDb25zdHMuamF2YQ==) | `85.71% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...op/ozone/container/keyvalue/KeyValueContainer.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVDb250YWluZXIuamF2YQ==) | `75.00% <ø> (-0.18%)` | `51.00 <0.00> (ø)` | |
   | [...one/container/keyvalue/KeyValueContainerCheck.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVDb250YWluZXJDaGVjay5qYXZh) | `75.36% <0.00%> (-0.73%)` | `21.00 <0.00> (-1.00)` | |
   | [...ava/org/apache/hadoop/hdds/utils/LevelDBStore.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy91dGlscy9MZXZlbERCU3RvcmUuamF2YQ==) | `72.34% <ø> (-1.42%)` | `38.00 <0.00> (-2.00)` | |
   | [...apache/hadoop/hdds/utils/LevelDBStoreIterator.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy91dGlscy9MZXZlbERCU3RvcmVJdGVyYXRvci5qYXZh) | `46.15% <ø> (-53.85%)` | `3.00 <0.00> (-3.00)` | |
   | [...apache/hadoop/hdds/utils/MetadataStoreBuilder.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy91dGlscy9NZXRhZGF0YVN0b3JlQnVpbGRlci5qYXZh) | `87.23% <60.00%> (-0.27%)` | `14.00 <0.00> (ø)` | |
   | [...zone/container/keyvalue/KeyValueContainerData.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVDb250YWluZXJEYXRhLmphdmE=) | `69.35% <100.00%> (+0.50%)` | `17.00 <0.00> (ø)` | |
   | [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `86.95% <0.00%> (-13.05%)` | `19.00% <0.00%> (-3.00%)` | |
   | ... and [18 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1166/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=footer). Last update [d85c7e3...6ff2338](https://codecov.io/gh/apache/hadoop-ozone/pull/1166?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#discussion_r452605910



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java
##########
@@ -186,8 +186,8 @@ private void checkContainerFile() throws IOException {
     }
 
     dbType = onDiskContainerData.getContainerDBType();
-    if (!dbType.equals(OZONE_METADATA_STORE_IMPL_ROCKSDB) &&
-        !dbType.equals(OZONE_METADATA_STORE_IMPL_LEVELDB)) {
+    if (!dbType.equals(CONTAINER_DB_TYPE_ROCKSDB) &&

Review comment:
       That makes sense to me. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#discussion_r452391946



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java
##########
@@ -186,8 +186,8 @@ private void checkContainerFile() throws IOException {
     }
 
     dbType = onDiskContainerData.getContainerDBType();
-    if (!dbType.equals(OZONE_METADATA_STORE_IMPL_ROCKSDB) &&
-        !dbType.equals(OZONE_METADATA_STORE_IMPL_LEVELDB)) {
+    if (!dbType.equals(CONTAINER_DB_TYPE_ROCKSDB) &&

Review comment:
       We are deprecating LevelDB support. I kept it there in case there are old containers using LevelDB.
   
   I am up for removing LevelDB support completely. I think it is safe to assume that there would not be a Container using LevelDB.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] vivekratnavel commented on pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#issuecomment-656860527


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


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1166: HDDS-3914. Remove LevelDB configuration option for DN Metastore

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1166:
URL: https://github.com/apache/hadoop-ozone/pull/1166#discussion_r451048561



##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestMetadataStore.java
##########
@@ -166,12 +170,12 @@ public void testIterator() throws Exception {
   public void testMetaStoreConfigDifferentFromType() throws IOException {
 
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.set(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL, storeImpl);
+
     String dbType;
     GenericTestUtils.setLogLevel(MetadataStoreBuilder.LOG, Level.DEBUG);
     GenericTestUtils.LogCapturer logCapturer =
         GenericTestUtils.LogCapturer.captureLogs(MetadataStoreBuilder.LOG);
-    if (storeImpl.equals(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL_LEVELDB)) {
+    if (storeImpl.equals(CONTAINER_DB_TYPE_LEVELDB)) {

Review comment:
       The logic needs to be reversed, maybe a typo. 

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java
##########
@@ -186,8 +186,8 @@ private void checkContainerFile() throws IOException {
     }
 
     dbType = onDiskContainerData.getContainerDBType();
-    if (!dbType.equals(OZONE_METADATA_STORE_IMPL_ROCKSDB) &&
-        !dbType.equals(OZONE_METADATA_STORE_IMPL_LEVELDB)) {
+    if (!dbType.equals(CONTAINER_DB_TYPE_ROCKSDB) &&

Review comment:
       Thanks @hanishakoneru  for working on this. Not sure if I understand correctly, since we are removing LevelDB support on DN, do we still need to define two types of DB for container here? 

##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestMetadataStore.java
##########
@@ -90,12 +93,12 @@ public void init() throws IOException {
         + "-" + storeImpl.toLowerCase());
 
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.set(OzoneConfigKeys.OZONE_METADATA_STORE_IMPL, storeImpl);
 
     store = MetadataStoreBuilder.newBuilder()
         .setConf(conf)
         .setCreateIfMissing(true)
         .setDbFile(testDir)
+        .setDBType(storeImpl)

Review comment:
       Do we still need this?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org