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/09/06 12:08:28 UTC

[GitHub] [ozone] sodonnel opened a new pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

sodonnel opened a new pull request #2615:
URL: https://github.com/apache/ozone/pull/2615


   ## What changes were proposed in this pull request?
   
   TestOzoneManagerListVolumes takes about 215 seconds to run, as it has several tests, and each tests creates a new mini-cluster:
   
   [INFO] Running org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes
   [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 215.633 s - in org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes
   
   We can refactor the test to use the same Mini-Cluster and just restart the OM component between each test, saving a lot of time.
   
   Note that this requires a small change in the OzoneManagerImpl class too.
   
   On my laptop the runtime went from 176 seconds -> 20 seconds.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5717
   
   ## How was this patch tested?
   
   Existing tests and a changes to existing 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] sodonnel commented on pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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


   @bharatviswa504 Are you happy with the updated commit? Are you ok for me to commit this now?


-- 
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] sodonnel merged pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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


   


-- 
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] bharatviswa504 commented on a change in pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -419,9 +419,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption)
     }
 
     loginOMUserIfSecurityEnabled(conf);
-
-    this.allowListAllVolumes = conf.getBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED,
-        OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT);
+    setInstanceVariablesFromConf();

Review comment:
       Minor: Similarly do we need  to remove isAclEnabled from OzoneManager L440




-- 
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] sodonnel commented on pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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


   The last run (which failed for some other reason) ran in:
   
   ```
   [INFO] Running org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes
   [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 49.395 s - in org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes
   ```
   
   Saving about 166 seconds.


-- 
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] sodonnel commented on pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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


   I am going to commit this now, as @bharatviswa504 gave a "LGTM" earlier pending one minor comment which I addressed. Let me know if you have any concerns and we can address them.


-- 
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] sodonnel commented on a change in pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -419,9 +419,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption)
     }
 
     loginOMUserIfSecurityEnabled(conf);
-
-    this.allowListAllVolumes = conf.getBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED,
-        OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT);
+    setInstanceVariablesFromConf();

Review comment:
       Ah, you are correct. Well spotted. I have fixed this and pushed another commit.




-- 
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] sodonnel commented on pull request #2615: HDDS-5717. Refactor TestOzoneManagerListVolumes to reuse mini-ozone clusters

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


   I spoke to @bharatviswa504 over slack - he is travelling this week, but gave me his +1 "offline", so I am going ahead with the commit.


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