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 2022/10/31 12:15:25 UTC

[GitHub] [ozone] symious opened a new pull request, #3918: HDDS-7441. Rename function names of retrieving metadata keys

symious opened a new pull request, #3918:
URL: https://github.com/apache/ozone/pull/3918

   ## What changes were proposed in this pull request?
   
   The current function names to get metadata keys in KeyValueContainerData might be confusing to users, this ticket is to update the names for better readability.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7441
   
   ## How was this patch tested?
   
   Only update function names, no need to add tests.


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] DaveTeng0 commented on pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #3918:
URL: https://github.com/apache/ozone/pull/3918#issuecomment-1298053493

   just curious~ what are some other containerTypes besides the _keyValueContainer_?


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] kerneltime commented on pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3918:
URL: https://github.com/apache/ozone/pull/3918#issuecomment-1297317346

   @SaketaChalamchala @aswinshakil @duongkame 


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] symious commented on a diff in pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3918:
URL: https://github.com/apache/ozone/pull/3918#discussion_r1010026674


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java:
##########
@@ -402,10 +403,10 @@ private void markBlocksForDeletionSchemaV1(
       try (BatchOperation batch = containerDB.getStore().getBatchHandler()
           .initBatchOperation()) {
         for (Long blkLong : delTX.getLocalIDList()) {
-          String blk = containerData.blockKey(blkLong);
+          String blk = containerData.getBlockMetaKey(blkLong);

Review Comment:
   I think the "meta" here is to indicate the key from Rocksdb, so that "MetaKey" can be used as common suffix for distinction.



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3918:
URL: https://github.com/apache/ozone/pull/3918#discussion_r1067777603


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java:
##########
@@ -402,10 +403,10 @@ private void markBlocksForDeletionSchemaV1(
       try (BatchOperation batch = containerDB.getStore().getBatchHandler()
           .initBatchOperation()) {
         for (Long blkLong : delTX.getLocalIDList()) {
-          String blk = containerData.blockKey(blkLong);
+          String blk = containerData.getBlockMetaKey(blkLong);

Review Comment:
   Hi @symious , thanks for improving the readability of the code.  
   I would recommend to use getBlockKey instead of getBlockMetaKey here since the function will return the key's value.  Meta is a word that different people may have different understandings. It will cause confusing. 



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi commented on pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3918:
URL: https://github.com/apache/ozone/pull/3918#issuecomment-1381376689

   +1. 


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] kerneltime commented on pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3918:
URL: https://github.com/apache/ozone/pull/3918#issuecomment-1297319233

   @GeorgeJahad 


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] ChenSammi merged pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
ChenSammi merged PR #3918:
URL: https://github.com/apache/ozone/pull/3918


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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] symious commented on a diff in pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3918:
URL: https://github.com/apache/ozone/pull/3918#discussion_r1068849834


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java:
##########
@@ -402,10 +403,10 @@ private void markBlocksForDeletionSchemaV1(
       try (BatchOperation batch = containerDB.getStore().getBatchHandler()
           .initBatchOperation()) {
         for (Long blkLong : delTX.getLocalIDList()) {
-          String blk = containerData.blockKey(blkLong);
+          String blk = containerData.getBlockMetaKey(blkLong);

Review Comment:
   Ok, removed the "meta" from function names, please have a look.



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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


[GitHub] [ozone] duongkame commented on a diff in pull request #3918: HDDS-7441. Rename function names of retrieving metadata keys

Posted by GitBox <gi...@apache.org>.
duongkame commented on code in PR #3918:
URL: https://github.com/apache/ozone/pull/3918#discussion_r1009692222


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/DeleteBlocksCommandHandler.java:
##########
@@ -402,10 +403,10 @@ private void markBlocksForDeletionSchemaV1(
       try (BatchOperation batch = containerDB.getStore().getBatchHandler()
           .initBatchOperation()) {
         for (Long blkLong : delTX.getLocalIDList()) {
-          String blk = containerData.blockKey(blkLong);
+          String blk = containerData.getBlockMetaKey(blkLong);

Review Comment:
   nit: why is it `BlockMetaKey` and not just `BlockKey`?



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

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

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


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