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/10 23:56:04 UTC
[GitHub] [hadoop-ozone] runzhiwang opened a new pull request #1053: HDDS-3737. Avoid serialization between UUID and String
runzhiwang opened a new pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053
## What changes were proposed in this pull request?
**What's the problem ?**
Serialization between UUID and String: UUID.toString (I have improved this) and UUID.fromString, not only cost cpu, because encode and decode String and UUID.fromString both cost cpu, but also make the proto bigger, because uuid is just a number which is 16Byte, covet it to string will need 32Byte.
**How to fix ?**
Actually, JDK implement UUID with two long number: `mostSigBits` and `leastSigBits`. When `UUID.fromString`, JDK get `mostSigBits` and `leastSigBits` from String, and new a object of UUID. So we can convert UUID to 2 long number in proto, which make serialization and de serialization UUID more faster, and make proto smaller.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3763
## 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 commented on a change in pull request #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#discussion_r447697031
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -301,14 +306,21 @@ public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
for (DatanodeDetailsProto member : pipeline.getMembersList()) {
nodes.add(DatanodeDetails.getFromProtoBuf(member));
}
+ UUID leaderId = null;
+ if (pipeline.hasLeaderID() &&
+ StringUtils.isNotEmpty(pipeline.getLeaderID())) {
+ leaderId = UUID.fromString(pipeline.getLeaderID());
Review comment:
@xiaoyuyao Thanks for review. I have updated the patch, Could you help review it again ?
----------------------------------------------------------------
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 #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#issuecomment-651668950
/pending @runzhiwang can you please add answer to the comment?
----------------------------------------------------------------
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 commented on a change in pull request #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#discussion_r441046058
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -301,14 +306,21 @@ public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
for (DatanodeDetailsProto member : pipeline.getMembersList()) {
nodes.add(DatanodeDetails.getFromProtoBuf(member));
}
+ UUID leaderId = null;
+ if (pipeline.hasLeaderID() &&
+ StringUtils.isNotEmpty(pipeline.getLeaderID())) {
+ leaderId = UUID.fromString(pipeline.getLeaderID());
Review comment:
Correct me if I'm wrong, with current approach, the improvement will be limited as fromProtoBuf to String will prefer process UUID before ID128. And to protoBuf will need to add both String version of UUID and the long/long version of uuid 128.
----------------------------------------------------------------
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 #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#issuecomment-651802478
/ready
----------------------------------------------------------------
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 #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#issuecomment-643017217
@elek @xiaoyuyao 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] runzhiwang commented on a change in pull request #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#discussion_r447544624
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -301,14 +306,21 @@ public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
for (DatanodeDetailsProto member : pipeline.getMembersList()) {
nodes.add(DatanodeDetails.getFromProtoBuf(member));
}
+ UUID leaderId = null;
+ if (pipeline.hasLeaderID() &&
+ StringUtils.isNotEmpty(pipeline.getLeaderID())) {
+ leaderId = UUID.fromString(pipeline.getLeaderID());
Review comment:
@xiaoyuyao Sorry for missing the comment. @elek Thanks for reminding.
> And to protoBuf will need to add both String version of UUID and the long/long version of uuid 128
This is for compatibility, in the future, I will remove String version of UUID.
> The improvement will be limited as fromProtoBuf to String will prefer process UUID before ID128
Do you mean we should prefer ID128 before process UUID.fromString ? If so, I think you are totally right, I will update it. Besides I will also remove UUID.fromString in the future.
----------------------------------------------------------------
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 a change in pull request #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#discussion_r447544624
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -301,14 +306,21 @@ public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
for (DatanodeDetailsProto member : pipeline.getMembersList()) {
nodes.add(DatanodeDetails.getFromProtoBuf(member));
}
+ UUID leaderId = null;
+ if (pipeline.hasLeaderID() &&
+ StringUtils.isNotEmpty(pipeline.getLeaderID())) {
+ leaderId = UUID.fromString(pipeline.getLeaderID());
Review comment:
@xiaoyuyao Sorry for missing the comment. @elek Thanks for reminding.
> And to protoBuf will need to add both String version of UUID and the long/long version of uuid 128
This is for compatibility, in the future, I will remove String version of UUID.
> The improvement will be limited as fromProtoBuf to String will prefer process UUID before ID128
Do you mean we should prefer ID128 before process UUID.fromString ? If so, I think you are totally right, I will update it. Besides I will also remove UUID.fromString in the future.
----------------------------------------------------------------
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] codecov-commenter commented on pull request #1053: [WIP]HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#issuecomment-643015878
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=h1) Report
> Merging [#1053](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/f7e95d9b015e764ca93cfe2ccfc96d95160931bc&el=desc) will **decrease** coverage by `0.10%`.
> The diff coverage is `84.09%`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1053 +/- ##
============================================
- Coverage 69.48% 69.38% -0.11%
- Complexity 9110 9113 +3
============================================
Files 961 961
Lines 48132 48154 +22
Branches 4672 4683 +11
============================================
- Hits 33446 33412 -34
- Misses 12468 12524 +56
Partials 2218 2218
```
| [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...rg/apache/hadoop/hdds/scm/pipeline/PipelineID.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmVJRC5qYXZh) | `74.07% <57.14%> (-20.05%)` | `11.00 <3.00> (ø)` | |
| [...g/apache/hadoop/hdds/protocol/DatanodeDetails.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9wcm90b2NvbC9EYXRhbm9kZURldGFpbHMuamF2YQ==) | `87.30% <71.42%> (-2.62%)` | `30.00 <6.00> (ø)` | |
| [.../org/apache/hadoop/hdds/scm/pipeline/Pipeline.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmUuamF2YQ==) | `84.47% <80.95%> (-1.04%)` | `46.00 <0.00> (+3.00)` | :arrow_down: |
| [...org/apache/hadoop/hdds/scm/XceiverClientRatis.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vWGNlaXZlckNsaWVudFJhdGlzLmphdmE=) | `79.06% <100.00%> (ø)` | `30.00 <0.00> (ø)` | |
| [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `95.91% <100.00%> (+20.40%)` | `23.00 <1.00> (+5.00)` | |
| [...a/org/apache/hadoop/ozone/HddsDatanodeService.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9IZGRzRGF0YW5vZGVTZXJ2aWNlLmphdmE=) | `50.93% <100.00%> (-0.23%)` | `30.00 <0.00> (ø)` | |
| [...ozone/container/common/helpers/DatanodeIdYaml.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL2hlbHBlcnMvRGF0YW5vZGVJZFlhbWwuamF2YQ==) | `100.00% <100.00%> (ø)` | `7.00 <0.00> (ø)` | |
| [...rg/apache/hadoop/hdds/scm/node/SCMNodeManager.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL25vZGUvU0NNTm9kZU1hbmFnZXIuamF2YQ==) | `87.28% <100.00%> (+0.10%)` | `55.00 <0.00> (ø)` | |
| [.../org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL29tL2hlbHBlcnMvT21LZXlJbmZvLmphdmE=) | `86.25% <100.00%> (+0.33%)` | `42.00 <0.00> (+2.00)` | |
| [...adoop/ozone/om/helpers/OmKeyLocationInfoGroup.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL29tL2hlbHBlcnMvT21LZXlMb2NhdGlvbkluZm9Hcm91cC5qYXZh) | `75.60% <100.00%> (+1.25%)` | `12.00 <2.00> (+1.00)` | |
| ... and [35 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1053/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=footer). Last update [53395a0...c9d2cb3](https://codecov.io/gh/apache/hadoop-ozone/pull/1053?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] runzhiwang commented on a change in pull request #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on a change in pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053#discussion_r447544624
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java
##########
@@ -301,14 +306,21 @@ public static Pipeline getFromProtobuf(HddsProtos.Pipeline pipeline)
for (DatanodeDetailsProto member : pipeline.getMembersList()) {
nodes.add(DatanodeDetails.getFromProtoBuf(member));
}
+ UUID leaderId = null;
+ if (pipeline.hasLeaderID() &&
+ StringUtils.isNotEmpty(pipeline.getLeaderID())) {
+ leaderId = UUID.fromString(pipeline.getLeaderID());
Review comment:
@xiaoyuyao Sorry for missing the comment. @elek Thanks for reminding.
> The improvement will be limited as fromProtoBuf to String will prefer process UUID before ID128
Do you mean we should prefer ID128 before process UUID.fromString ? If so, I think you are totally right, I will update it. Besides I will also remove UUID.fromString in the future.
> And to protoBuf will need to add both String version of UUID and the long/long version of uuid 128
This is for compatibility, in the future, I will remove String version of UUID.
----------------------------------------------------------------
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 #1053: HDDS-3737. Avoid serialization between UUID and String
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1053:
URL: https://github.com/apache/hadoop-ozone/pull/1053
----------------------------------------------------------------
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