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/09/03 01:49:54 UTC

[GitHub] [hadoop-ozone] runzhiwang opened a new pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   
   ## What changes were proposed in this pull request?
   
   **What's the problem ?**
   
   ![image](https://user-images.githubusercontent.com/51938049/92061322-87742780-edc8-11ea-80f9-7521a18b1ac4.png)
   
   **What's the reason ?**
   
   we can not make sure `omKeyInfo.getCreationTime()` equals to ` omKeyInfo.getModificationTime()`
   
   1. the creationTime was set at [TestOMRequestUtils.addKeyToTable](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L68) -> [OmKeyInfo#createOmKeyInfo](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/TestOMRequestUtils.java#L253)
   2. the modificationTime was set in KeyArgs at [doPreExecute(createAllocateBlockRequest())](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L58) -> [OMRequest#preExecute](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java#L115). 
    And the KeyArgs.modificationTime was set in OmKeyInfo at [omAllocateBlockRequest.validateAndUpdateCache](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L84) -> [openKeyInfo.setModificationTime](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java#L203).
   
   3. Between [doPreExecute(createAllocateBlockRequest())](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L58) and [TestOMRequestUtils.addKeyToTable](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L68), there is time consuming code, so we can not make sure the millisecond time is different.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4199
   
   ## How was this patch tested?
   
   no need new test.
   


----------------------------------------------------------------
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] runzhiwang commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   @bharatviswa504 CI passed. Could you help merge this patch ? Thank you very much.


----------------------------------------------------------------
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] runzhiwang commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   @bharatviswa504 Could you help review this patch ? Thank you very much.


----------------------------------------------------------------
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] amaliujia commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   In fact, greater than or equal to is a good idea!


----------------------------------------------------------------
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] adoroszlai merged pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   


----------------------------------------------------------------
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] runzhiwang edited a comment on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

Posted by GitBox <gi...@apache.org>.
runzhiwang edited a comment on pull request #1379:
URL: https://github.com/apache/hadoop-ozone/pull/1379#issuecomment-686290328


   @bharatviswa504 @amaliujia Thanks for review. I have updated the patch. Besides, I change the order of [doPreExecute(createAllocateBlockRequest())](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L58) and [TestOMRequestUtils.addKeyToTable](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L68), to make sure `creationTime <= modificationTime`, otherwise `creationTime >= modificationTime` looks weird.


----------------------------------------------------------------
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] amaliujia commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   Thanks for the investigation!
   
   I am not sure if this is a good practice: will it help to insert a wait/sleep/a random operation after `doPreExecute(createAllocateBlockRequest());` to create the opportunity to differentiate  creation time and modification time  


----------------------------------------------------------------
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] amaliujia edited a comment on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

Posted by GitBox <gi...@apache.org>.
amaliujia edited a comment on pull request #1379:
URL: https://github.com/apache/hadoop-ozone/pull/1379#issuecomment-686241247


   Thanks for the investigation!
   
   I am not sure if this is a good practice: will it help to insert a wait/sleep/a random operation after `doPreExecute(createAllocateBlockRequest());` to create the opportunity to differentiate  creation time and modification?time  


----------------------------------------------------------------
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] runzhiwang commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   @bharatviswa504 @amaliujia Thanks for review. I have updated the patch. Besides, I change the order of [doPreExecute(createAllocateBlockRequest())](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L58) and [TestOMRequestUtils.addKeyToTable](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java#L68), to make sure creationTime <= modificationTime, otherwise creationTime >= modificationTime looks weird.


----------------------------------------------------------------
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] amaliujia commented on a change in pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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



##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMAllocateBlockRequest.java
##########
@@ -97,8 +95,11 @@ public void testValidateAndUpdateCache() throws Exception {
     // Check modification time
     Assert.assertEquals(modifiedOmRequest.getAllocateBlockRequest()
         .getKeyArgs().getModificationTime(), omKeyInfo.getModificationTime());
-    Assert.assertNotEquals(omKeyInfo.getCreationTime(),
-        omKeyInfo.getModificationTime());
+
+    // creationTime was assigned at TestOMRequestUtils.addKeyToTable
+    // modificationTime was assigned at doPreExecute(createAllocateBlockRequest())
+    Assert.assertTrue(
+        omKeyInfo.getCreationTime() <= omKeyInfo.getModificationTime());

Review comment:
       +1 LGTM!




----------------------------------------------------------------
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] adoroszlai commented on pull request #1379: HDDS-4199. Fix failed UT: TestOMAllocateBlockRequest#testValidateAndUpdateCache

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


   Thanks @runzhiwang for fixing this, @amaliujia and @bharatviswa504 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