You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tubemq.apache.org by GitBox <gi...@apache.org> on 2020/06/04 05:04:09 UTC
[GitHub] [incubator-tubemq] Technoboy- opened a new pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Technoboy- opened a new pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125
----------------------------------------------------------------
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
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125#discussion_r435272490
##########
File path: tubemq-core/src/main/java/org/apache/tubemq/corebase/utils/CompressionType.java
##########
@@ -0,0 +1,94 @@
+/**
Review comment:
Compression processing and compression enumeration need to be separated: at present, there is no problem when there is only one compression method, but subsequent compression methods are added, some methods need to be initialized, and some require other processing, the enumeration class will become unstable, and need to be adjusted frequently.
So enumerating compression methods and specific compression classification will be better to solve this problem
----------------------------------------------------------------
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
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125#discussion_r435046493
##########
File path: tubemq-client/src/main/java/org/apache/tubemq/client/producer/SimpleMessageProducer.java
##########
@@ -335,11 +336,13 @@ private void checkMessageAndStatus(final Message message) throws TubeClientExcep
}
private byte[] encodePayload(final Message message) {
Review comment:
It would be better to put the message compression operation in the Message class. It is open to the user and can avoid the situation where the Message data has been compressed when it is sent and then compressed 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
[GitHub] [incubator-tubemq] Technoboy- closed pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
Technoboy- closed pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125
----------------------------------------------------------------
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
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125#discussion_r435276102
##########
File path: tubemq-core/src/main/java/org/apache/tubemq/corebase/utils/DataConverterUtil.java
##########
@@ -233,8 +233,17 @@
payloadDataLen -= attrLen;
}
}
- final byte[] payload = new byte[payloadDataLen];
+ byte[] payload = new byte[payloadDataLen];
System.arraycopy(payloadData.array(), readPos, payload, 0, payloadDataLen);
+ String compressionType = "";
Review comment:
The extraction of the compression type value needs to be performed by calling the decoding function of the Message attribute. With the iteration of the version, it may be changed later, and it is more appropriate to process it by the method
----------------------------------------------------------------
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
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125#discussion_r435284842
##########
File path: tubemq-client/src/main/java/org/apache/tubemq/client/producer/SimpleMessageProducer.java
##########
@@ -321,7 +322,7 @@ private void checkMessageAndStatus(final Message message) throws TubeClientExcep
builder.setTopicName(partition.getTopic());
builder.setPartitionId(partition.getPartitionId());
builder.setData(ByteString.copyFrom(encodePayload(message)));
- builder.setFlag(MessageFlagUtils.getFlag(message));
+ builder.setFlag(MessageFlagUtils.getFlag(message, true));
Review comment:
The function of payload data process is recommended to be placed on line 321, and the attribute need to be adjusted by the function,
and there is no need to add a mandatory flag setting, which will make the maintenance personnel confused later.
----------------------------------------------------------------
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
[GitHub] [incubator-tubemq] gosonzhang commented on a change in pull request #125: [TUBEMQ-185] Improve producer implementation - support data compression
Posted by GitBox <gi...@apache.org>.
gosonzhang commented on a change in pull request #125:
URL: https://github.com/apache/incubator-tubemq/pull/125#discussion_r435278576
##########
File path: tubemq-client/src/main/java/org/apache/tubemq/client/producer/SimpleMessageProducer.java
##########
@@ -335,11 +336,13 @@ private void checkMessageAndStatus(final Message message) throws TubeClientExcep
}
private byte[] encodePayload(final Message message) {
- final byte[] payload = message.getData();
- final String attribute = message.getAttribute();
+ byte[] payload = message.getData();
+ String attribute = message.getAttribute();
if (TStringUtils.isBlank(attribute)) {
- return payload;
+ attribute = "";
}
+ attribute = attribute + "," + TokenConstants.TOKEN_COMPRESS_TYPE + producerConfig.getCompressionType().name();
Review comment:
The attribute item in the attribute needs to be added in a=b, c=d mode and has a fixed format. It is best to call the method of Message to add the attribute item in this place, instead of seeming to manually stitch together to avoid incompatibility after adjusting the encoding
----------------------------------------------------------------
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