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/06/08 07:21:56 UTC

[GitHub] [hadoop-ozone] runzhiwang opened a new pull request #1034: HDDS-3749. Improve OM performance with 3% by avoid stream.collect

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


   ## What changes were proposed in this pull request?
   
   I start a ozone cluster with 1000 datanodes and 10 s3gateway, and run two weeks with heavy workload, and perf om. There are 3 places use stream.collect which cost 3% cpu.
   
   1. stream.collect in Pipeline::getFromProtobuf
   ![image](https://user-images.githubusercontent.com/51938049/84002528-255b8000-a99b-11ea-9aed-25f5fd8eaa92.png)
   
   2. stream.collect in OmKeyInfo::getProtobuf
   ![image](https://user-images.githubusercontent.com/51938049/84002623-4cb24d00-a99b-11ea-8acb-e485efcba658.png)
   ![image](https://user-images.githubusercontent.com/51938049/84002730-74091a00-a99b-11ea-9da6-046a3145d01d.png)
   
   3. stream.collect in OMKeyCommitRequest::validateAndUpdateCache
   ![image](https://user-images.githubusercontent.com/51938049/84002874-b5012e80-a99b-11ea-89fb-0df6fb731e5b.png)
   
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3749
   
   ## How was this patch tested?
   
   Existed 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.

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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @elek @bharatviswa504    I think the reason there are code1 and code2 both use java.util.stream.Collectors.   In this patch, I remove code1, and trigger CI, because code2 still use java.util.stream.Collectors, CI does not fail. Then, another commit remove code2 and merged, but this patch does not trigger CI again, if trigger, CI will fail, because both code1 and code2 were removed, there are no code use java.util.stream.Collectors. So when merge this patch, CI fail.   Sorry for that.


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @elek @bharatviswa504    I think the reason is there are code1 and code2 both use `java.util.stream.Collectors`, PR1 remove code1, PR2 remove code2.   When submit PR1, PR2 has not been merged, and code2 still exist in master and use `java.util.stream.Collectors`,  so PR1 pass CI. And then PR2 was merged, both code1 and code2 were removed, but PR1 does not triggger CI again, if trigger, CI will fail, because both code1 and code2 were removed, there are no code use `java.util.stream.Collectors`. So when merge PR1, CI fail.   Sorry for that.


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @elek @bharatviswa504    I think the reason is there are code1 and code2 both use `java.util.stream.Collectors`, PR1 remove code1, PR2 remove code2.   When submit PR1, PR2 has not been merged, and code2 still exist in master and use `java.util.stream.Collectors`,  so PR1 pass CI. And then PR2 was merged before PR1, both code1 and code2 were removed, but PR1 does not triggger CI again, if trigger, CI will fail, because both code1 and code2 were removed, there are no code use `java.util.stream.Collectors`. So when merge PR1, CI fail.   Sorry for that.


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @elek @bharatviswa504    I think the reason is there are code1 and code2 both use `java.util.stream.Collectors`, PR1 remove code1, PR2 remove code2.   When submit PR1, PR2 has not been merged, and code2 still exist in master and use `java.util.stream.Collectors`,  so PR1 pass CI. Then PR2 was merged before PR1, both code1 and code2 were removed, but PR1 does not triggger CI again, if trigger, CI will fail, because both code1 and code2 were removed, there are no code use `java.util.stream.Collectors`. So when merge PR1, CI fail.   Sorry for that.


----------------------------------------------------------------
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] nandakumar131 commented on a change in pull request #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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



##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -411,13 +415,19 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) {
     if (keyInfo == null) {
       return null;
     }
+
+    List<OmKeyLocationInfoGroup> omKeyLocationInfos = new ArrayList<>();
+    List<KeyLocationList> keyLocationLists = keyInfo.getKeyLocationListList();
+    for (KeyLocationList keyLocationList : keyLocationLists) {

Review comment:
       Temporary variable `keyLocationLists` can be removed.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -289,14 +290,20 @@ public boolean isEmpty() {
   public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
       throws UnknownPipelineStateException {
     Preconditions.checkNotNull(pipeline, "Pipeline is null");
+
+    List<DatanodeDetails> nodes = new ArrayList<>();
+    List<DatanodeDetailsProto> membersList = pipeline.getMembersList();
+    for (DatanodeDetailsProto member : membersList) {

Review comment:
       We don't need the temporary variable `memberList`.
   It can be replaced with
   ```
   for (DatanodeDetailsProto member : pipeline.getMembersList())
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -132,10 +134,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           keyName, IAccessAuthorizer.ACLType.WRITE,
           commitKeyRequest.getClientID());
 
-      List<OmKeyLocationInfo> locationInfoList = commitKeyArgs
-          .getKeyLocationsList().stream()
-          .map(OmKeyLocationInfo::getFromProtobuf)
-          .collect(Collectors.toList());
+      List<OmKeyLocationInfo> locationInfoList = new ArrayList<>();
+      List<KeyLocation> keyLocations = commitKeyArgs
+          .getKeyLocationsList();
+
+      for (KeyLocation keyLocation : keyLocations) {

Review comment:
       The temporary variable `keyLocations ` can be removed.




----------------------------------------------------------------
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 merged pull request #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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






----------------------------------------------------------------
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] elek commented on pull request #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   Thanks the quick fix @bharatviswa504. 
   
   Do you have any idea how was it possible? The last build seems to be green... What should we improve on our process to avoid some problems?


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @nandakumar131 Thanks for review. I have updated.


----------------------------------------------------------------
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 #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   @elek @bharatviswa504    I think the reason there are code1 and code2 both use java.util.stream.Collectors.   In this patch, I remove code1, and trigger CI, because code2 still use java.util.stream.Collectors, CI does not fail. Then, another commit remove code2, but this patch does not trigger CI again, if trigger, CI will fail, because both code1 and code2 were removed, there are no code use java.util.stream.Collectors. So when merge this patch, CI fail.   Sorry for that.


----------------------------------------------------------------
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] bharatviswa504 commented on pull request #1034: HDDS-3749. Improve OM performance with 3.7% by avoid stream.collect

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


   Hi @xiaoyuyao 
   This has caused checkstyle issues and PR's are failing with CI run.
   Posted a PR to fix this
   https://github.com/apache/hadoop-ozone/pull/1052
   


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