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 2021/03/12 17:08:35 UTC

[GitHub] [ozone] rakeshadr opened a new pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

rakeshadr opened a new pull request #2035:
URL: https://github.com/apache/ozone/pull/2035


   ## What changes were proposed in this pull request?
   
   This task is to cleanup New FileTables when the request is not successful in the following requests.
   ```
   -> OMAllocateBlockRequestV1,
   -> OMFileCreateRequestV1,
   -> OMKeyCreateRequestV1,
   -> OMKeyRenameRequestV1,
   -> S3InitiateMultipartUploadRequestV1
   ```
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4973
   
   ## How was this patch tested?
   
   Existing UTs.
   


----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] rakeshadr commented on pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2035:
URL: https://github.com/apache/ozone/pull/2035#issuecomment-797860567


   Good catch! @bharatviswa504 , its copy paste prob:-) I have addressed the same in all the places.


----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] rakeshadr merged pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #2035:
URL: https://github.com/apache/ozone/pull/2035


   


----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2035:
URL: https://github.com/apache/ozone/pull/2035#discussion_r593551350



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3InitiateMultipartUploadResponseV1.java
##########
@@ -54,6 +54,15 @@ public S3InitiateMultipartUploadResponseV1(
     this.parentDirInfos = parentDirInfos;
   }
 
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public S3InitiateMultipartUploadResponseV1(@Nonnull OMResponse omResponse) {
+    super(omResponse);
+    checkStatusNotOK();

Review comment:
       We don't need checkStatusNotOK for this, as super has already that check.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponseV1.java
##########
@@ -53,6 +53,15 @@ public OMFileCreateResponseV1(@Nonnull OMResponse omResponse,
     this.parentDirInfos = parentDirInfos;
   }
 
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMFileCreateResponseV1(@Nonnull OMResponse omResponse) {
+    super(omResponse);

Review comment:
       Same here

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCreateResponseV1.java
##########
@@ -45,4 +45,13 @@ public OMKeyCreateResponseV1(@Nonnull OMResponse omResponse,
     super(omResponse, omKeyInfo, parentDirInfos, openKeySessionID,
             omBucketInfo);
   }
+
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMKeyCreateResponseV1(@Nonnull OMResponse omResponse) {
+    super(omResponse);

Review comment:
       Same here

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMAllocateBlockResponseV1.java
##########
@@ -43,6 +43,15 @@ public OMAllocateBlockResponseV1(@Nonnull OMResponse omResponse,
     super(omResponse, omKeyInfo, clientID, omBucketInfo);
   }
 
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMAllocateBlockResponseV1(@Nonnull OMResponse omResponse) {
+    super(omResponse);

Review comment:
       Same here

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyRenameResponseV1.java
##########
@@ -46,6 +46,15 @@ public OMKeyRenameResponseV1(@Nonnull OMResponse omResponse,
     this.isRenameDirectory = isRenameDirectory;
   }
 
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMKeyRenameResponseV1(@Nonnull OMResponse omResponse) {
+    super(omResponse);

Review comment:
       We don't need checkStatusNotOK for this, as super has already that check.




----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] rakeshadr commented on pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2035:
URL: https://github.com/apache/ozone/pull/2035#issuecomment-797874293


   Thanks @bharatviswa504 for the useful review 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] rakeshadr commented on a change in pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2035:
URL: https://github.com/apache/ozone/pull/2035#discussion_r593546433



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponseV1.java
##########
@@ -33,12 +33,14 @@
 import java.util.List;
 
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
 
 /**
  * Response for create file request layout version V1.
  */
-@CleanupTableInfo(cleanupTables = {DIRECTORY_TABLE, OPEN_FILE_TABLE})
+@CleanupTableInfo(cleanupTables = {DIRECTORY_TABLE, FILE_TABLE,

Review comment:
       Thanks @bharatviswa504 , agreed. I've removed 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2035: HDDS-4973. [FSO] Missed to cleanup new FileTables in OMRequests

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2035:
URL: https://github.com/apache/ozone/pull/2035#discussion_r593359850



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponseV1.java
##########
@@ -33,12 +33,14 @@
 import java.util.List;
 
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
 
 /**
  * Response for create file request layout version V1.
  */
-@CleanupTableInfo(cleanupTables = {DIRECTORY_TABLE, OPEN_FILE_TABLE})
+@CleanupTableInfo(cleanupTables = {DIRECTORY_TABLE, FILE_TABLE,

Review comment:
       Why do we need cleanup of file table, wheen we add entries to open file table and directory table/




----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org