You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/04/26 08:45:15 UTC
[GitHub] [rocketmq] ferrirW opened a new pull request, #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
ferrirW opened a new pull request, #4213:
URL: https://github.com/apache/rocketmq/pull/4213
## What is the purpose of the change
refer to https://github.com/apache/rocketmq/issues/4203
## Brief changelog
1. Add zstd / lz4 compressor
2. fix some spelling error
## Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily. Notice, `it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR`.
- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/rocketmq/tree/master/test).
- [x] Run `mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle` to make sure basic checks pass. Run `mvn clean install -DskipITs` to make sure unit-test pass. Run `mvn clean test-compile failsafe:integration-test` to make sure integration-test pass.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1110907649
[](https://coveralls.io/builds/48616846)
Coverage decreased (-0.1%) to 52.022% when pulling **92846761fa42ecdfbda10bffd8e0f8be58159888 on ferrirW:ROCKETMQ-4203** into **b6e65a8054a60cb5c62f5ade8a9e3927cf157185 on apache:develop**.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1111728362
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#4213](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5584a78) into [develop](https://codecov.io/gh/apache/rocketmq/commit/b6e65a8054a60cb5c62f5ade8a9e3927cf157185?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6e65a8) will **decrease** coverage by `0.12%`.
> The diff coverage is `71.42%`.
```diff
@@ Coverage Diff @@
## develop #4213 +/- ##
=============================================
- Coverage 48.05% 47.93% -0.13%
+ Complexity 5051 5045 -6
=============================================
Files 637 642 +5
Lines 42640 42793 +153
Branches 5600 5603 +3
=============================================
+ Hits 20492 20511 +19
- Misses 19635 19770 +135
+ Partials 2513 2512 -1
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.87% <0.00%> (-0.05%)` | :arrow_down: |
| [...client/trace/hook/EndTransactionTraceHookImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvdHJhY2UvaG9vay9FbmRUcmFuc2FjdGlvblRyYWNlSG9va0ltcGwuamF2YQ==) | `13.33% <0.00%> (ø)` | |
| [...c/main/java/org/apache/rocketmq/common/MixAll.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vTWl4QWxsLmphdmE=) | `44.44% <ø> (ø)` | |
| [.../main/java/org/apache/rocketmq/common/UtilAll.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vVXRpbEFsbC5qYXZh) | `42.23% <ø> (ø)` | |
| [...apache/rocketmq/common/message/MessageDecoder.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRGVjb2Rlci5qYXZh) | `80.58% <0.00%> (-0.60%)` | :arrow_down: |
| [...mq/client/impl/producer/DefaultMQProducerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9wcm9kdWNlci9EZWZhdWx0TVFQcm9kdWNlckltcGwuamF2YQ==) | `44.70% <60.00%> (-0.02%)` | :arrow_down: |
| [...he/rocketmq/common/compression/ZlibCompressor.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vY29tcHJlc3Npb24vWmxpYkNvbXByZXNzb3IuamF2YQ==) | `73.33% <73.33%> (ø)` | |
| [...e/rocketmq/common/compression/CompressionType.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vY29tcHJlc3Npb24vQ29tcHJlc3Npb25UeXBlLmphdmE=) | `73.91% <73.91%> (ø)` | |
| [...che/rocketmq/common/compression/Lz4Compressor.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vY29tcHJlc3Npb24vTHo0Q29tcHJlc3Nvci5qYXZh) | `78.37% <78.37%> (ø)` | |
| [...he/rocketmq/common/compression/ZstdCompressor.java](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vY29tcHJlc3Npb24vWnN0ZENvbXByZXNzb3IuamF2YQ==) | `78.37% <78.37%> (ø)` | |
| ... and [29 more](https://codecov.io/gh/apache/rocketmq/pull/4213/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a492592...5584a78](https://codecov.io/gh/apache/rocketmq/pull/4213?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hill007299 merged pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hill007299 merged PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ferrirW commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
ferrirW commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859511470
##########
common/src/main/java/org/apache/rocketmq/common/compression/CompressorFactory.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+public class CompressorFactory {
+
+ private static final Lz4Compressor LZ4_COMPRESSOR = new Lz4Compressor();
+ private static final ZlibCompressor ZLIB_COMPRESSOR = new ZlibCompressor();
+ private static final ZstdCompressor ZSTD_COMPRESSOR = new ZstdCompressor();
+
Review Comment:
> finally assign one of them as default.
I think init of compressType has finish this default assign. And may be I shoule use a enummap to store compressor.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859753800
##########
common/src/main/java/org/apache/rocketmq/common/compression/CompressorFactory.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+public class CompressorFactory {
+
+ private static final Lz4Compressor LZ4_COMPRESSOR = new Lz4Compressor();
+ private static final ZlibCompressor ZLIB_COMPRESSOR = new ZlibCompressor();
+ private static final ZstdCompressor ZSTD_COMPRESSOR = new ZstdCompressor();
+
Review Comment:
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.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hill007299 commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hill007299 commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1119559985
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.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] HScarb commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
HScarb commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859318324
##########
common/src/main/java/org/apache/rocketmq/common/sysflag/MessageSysFlag.java:
##########
@@ -16,7 +16,21 @@
*/
package org.apache.rocketmq.common.sysflag;
+import org.apache.rocketmq.common.compression.CompressionType;
+
public class MessageSysFlag {
+
+ /**
+ * Meaning of each bit in the system flag
+ *
+ * | bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
Review Comment:
Start from 0 to 7 would be better to read
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r858613176
##########
common/src/main/java/org/apache/rocketmq/common/compression/CompressorFactory.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+public class CompressorFactory {
+
+ private static final Lz4Compressor LZ4_COMPRESSOR = new Lz4Compressor();
+ private static final ZlibCompressor ZLIB_COMPRESSOR = new ZlibCompressor();
+ private static final ZstdCompressor ZSTD_COMPRESSOR = new ZstdCompressor();
+
Review Comment:
You should use a Compressor[] to hold these different Compressors, and assign an id number to each Compressor, and finally assign one of them as default.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ferrirW commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
ferrirW commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859488047
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageDecoder.java:
##########
@@ -163,7 +166,8 @@ public static byte[] encode(MessageExt messageExt, boolean needCompress) throws
int storehostAddressLength = (sysFlag & MessageSysFlag.STOREHOSTADDRESS_V6_FLAG) == 0 ? 8 : 20;
byte[] newBody = messageExt.getBody();
if (needCompress && (sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG) {
- newBody = UtilAll.compress(body, 5);
+ Compressor compressor = CompressorFactory.getCompressor(MessageSysFlag.getCompressionType(sysFlag));
+ newBody = compressor.compress(body, 5);
Review Comment:
I don't know why it was 5 at the first, so I think it's better to keep things as they are, and avoid any compatibility issues.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] HScarb commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
HScarb commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859316822
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageDecoder.java:
##########
@@ -163,7 +166,8 @@ public static byte[] encode(MessageExt messageExt, boolean needCompress) throws
int storehostAddressLength = (sysFlag & MessageSysFlag.STOREHOSTADDRESS_V6_FLAG) == 0 ? 8 : 20;
byte[] newBody = messageExt.getBody();
if (needCompress && (sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG) {
- newBody = UtilAll.compress(body, 5);
+ Compressor compressor = CompressorFactory.getCompressor(MessageSysFlag.getCompressionType(sysFlag));
+ newBody = compressor.compress(body, 5);
Review Comment:
Do we need to get compress level from `System.getProperty(MixAll.MESSAGE_COMPRESS_LEVEL, "5")`?
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ferrirW commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
ferrirW commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859448995
##########
common/src/main/java/org/apache/rocketmq/common/sysflag/MessageSysFlag.java:
##########
@@ -16,7 +16,21 @@
*/
package org.apache.rocketmq.common.sysflag;
+import org.apache.rocketmq.common.compression.CompressionType;
+
public class MessageSysFlag {
+
+ /**
+ * Meaning of each bit in the system flag
+ *
+ * | bit | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
Review Comment:
> Start from 0 to 7 would be better to read
I think it would be better to be consistent with the actual physical order
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r858539536
##########
common/src/main/java/org/apache/rocketmq/common/compression/ZstdCompressor.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+import com.github.luben.zstd.ZstdInputStream;
+import com.github.luben.zstd.ZstdOutputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
+
+/**
+ * @author wangfan8@xiaomi.com
+ * @date 2022/4/25
+ */
Review Comment:
Don't leave the author info
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] HScarb commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
HScarb commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859314565
##########
common/src/main/java/org/apache/rocketmq/common/UtilAll.java:
##########
@@ -295,6 +295,7 @@ private static byte charToByte(char c) {
return (byte) "0123456789ABCDEF".indexOf(c);
}
+ @Deprecated
Review Comment:
It's better to add link to new method, like this
```java
/**
* @deprecated use {@link Compressor#decompress()} instead.
*/
```
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ferrirW commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
ferrirW commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859450753
##########
common/src/main/java/org/apache/rocketmq/common/compression/ZstdCompressor.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+import com.github.luben.zstd.ZstdInputStream;
+import com.github.luben.zstd.ZstdOutputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import org.apache.rocketmq.common.constant.LoggerName;
+import org.apache.rocketmq.logging.InternalLogger;
+import org.apache.rocketmq.logging.InternalLoggerFactory;
+
+/**
+ * @author wangfan8@xiaomi.com
+ * @date 2022/4/25
+ */
Review Comment:
thanks for reminding
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] tianliuliu commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
tianliuliu commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r861637010
##########
common/src/main/java/org/apache/rocketmq/common/message/MessageDecoder.java:
##########
@@ -163,7 +166,8 @@ public static byte[] encode(MessageExt messageExt, boolean needCompress) throws
int storehostAddressLength = (sysFlag & MessageSysFlag.STOREHOSTADDRESS_V6_FLAG) == 0 ? 8 : 20;
byte[] newBody = messageExt.getBody();
if (needCompress && (sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG) {
- newBody = UtilAll.compress(body, 5);
+ Compressor compressor = CompressorFactory.getCompressor(MessageSysFlag.getCompressionType(sysFlag));
+ newBody = compressor.compress(body, 5);
Review Comment:
I also think System.getProperty(MixAll.MESSAGE_COMPRESS_LEVEL, "5") is better
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r858543053
##########
common/src/main/java/org/apache/rocketmq/common/compression/Compressor.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+import java.io.IOException;
+
+public interface Compressor {
+
+ public byte[] compress(byte[] src, int level) throws IOException;
Review Comment:
remove public and add a comment for this method
##########
common/src/main/java/org/apache/rocketmq/common/compression/Compressor.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.common.compression;
+
+import java.io.IOException;
+
+public interface Compressor {
+
+ public byte[] compress(byte[] src, int level) throws IOException;
+
+ public byte[] decompress(byte[] src) throws IOException;
Review Comment:
remove public and add a comment for this 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.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1111567229
Nice try
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] HScarb commented on a diff in pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
HScarb commented on code in PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#discussion_r859314709
##########
common/src/main/java/org/apache/rocketmq/common/UtilAll.java:
##########
@@ -335,6 +336,7 @@ public static byte[] uncompress(final byte[] src) throws IOException {
return result;
}
+ @Deprecated
Review Comment:
It's better to add link to new method, like this
/**
* @deprecated use {@link Compressor#compress()} instead.
*/
##########
common/src/main/java/org/apache/rocketmq/common/UtilAll.java:
##########
@@ -335,6 +336,7 @@ public static byte[] uncompress(final byte[] src) throws IOException {
return result;
}
+ @Deprecated
Review Comment:
It's better to add link to new method, like this
```java
/**
* @deprecated use {@link Compressor#compress()} instead.
*/
```
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ferrirW commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
ferrirW commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1119489028
@duhenglucky Hi, can you help review this pr and check if this can be merged?
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] duhenglucky commented on pull request #4213: [ISSUE #4203] Support zstd/lz4 compression to improve send/receive performance
Posted by GitBox <gi...@apache.org>.
duhenglucky commented on PR #4213:
URL: https://github.com/apache/rocketmq/pull/4213#issuecomment-1119557457
>
copy
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org