You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by lizhanhui <gi...@git.apache.org> on 2017/04/06 15:41:43 UTC
[GitHub] incubator-rocketmq pull request #89: [ROCKETMQ-166] onException callback may...
GitHub user lizhanhui opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/89
[ROCKETMQ-166] onException callback may capture compressed message body
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lizhanhui/incubator-rocketmq ROCKETMQ-166
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/89.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #89
----
commit 638a692b9540f31768b76d43936a3ca277138ae1
Author: Zhanhui Li <li...@apache.org>
Date: 2017-04-06T15:39:33Z
Fix ROCKETMQ-166
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #89: [ROCKETMQ-166] onException callback may...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/89#discussion_r115650526
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -306,6 +310,19 @@ public SendResult sendMessage(//
final SendMessageContext context, // 11
final DefaultMQProducerImpl producer // 12
) throws RemotingException, MQBrokerException, InterruptedException {
+
+ byte[] msgBody = msg.getBody();
+ Integer sysFlag = requestHeader.getSysFlag();
+ if (null != sysFlag && ((sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG)) {
+ try {
+ msgBody = UtilAll.compress(msgBody, zipCompressLevel);
+ } catch (IOException e) {
+ requestHeader.setSysFlag(MessageSysFlag.clearCompressedFlag(sysFlag));
+ log.error("tryToCompressMessage exception", e);
--- End diff --
I just wonder why both WARN and ERROR are needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #89: [ROCKETMQ-166] onException callback may...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/89#discussion_r111708529
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -306,6 +310,19 @@ public SendResult sendMessage(//
final SendMessageContext context, // 11
final DefaultMQProducerImpl producer // 12
) throws RemotingException, MQBrokerException, InterruptedException {
+
+ byte[] msgBody = msg.getBody();
+ Integer sysFlag = requestHeader.getSysFlag();
+ if (null != sysFlag && ((sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG)) {
+ try {
+ msgBody = UtilAll.compress(msgBody, zipCompressLevel);
+ } catch (IOException e) {
+ requestHeader.setSysFlag(MessageSysFlag.clearCompressedFlag(sysFlag));
+ log.error("tryToCompressMessage exception", e);
--- End diff --
What is the reason to have ERROR and WARN here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #89: [ROCKETMQ-166] onException callback may...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/89#discussion_r115651498
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -306,6 +310,19 @@ public SendResult sendMessage(//
final SendMessageContext context, // 11
final DefaultMQProducerImpl producer // 12
) throws RemotingException, MQBrokerException, InterruptedException {
+
+ byte[] msgBody = msg.getBody();
+ Integer sysFlag = requestHeader.getSysFlag();
+ if (null != sysFlag && ((sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG)) {
+ try {
+ msgBody = UtilAll.compress(msgBody, zipCompressLevel);
+ } catch (IOException e) {
+ requestHeader.setSysFlag(MessageSysFlag.clearCompressedFlag(sysFlag));
+ log.error("tryToCompressMessage exception", e);
--- End diff --
Got your point.
1. The code is moved here, and same log level should be applied IMO. @vongosling @vintagewang any particular reason behind this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #89: [ROCKETMQ-166] onException callback may captur...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/89
Comment to bump up this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #89: [ROCKETMQ-166] onException callback may captur...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/89
[](https://coveralls.io/builds/10964577)
Coverage increased (+0.04%) to 31.927% when pulling **638a692b9540f31768b76d43936a3ca277138ae1 on lizhanhui:ROCKETMQ-166** into **45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #89: [ROCKETMQ-166] onException callback may captur...
Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/89
@shroman @stevenschew
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #89: [ROCKETMQ-166] onException callback may...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/89#discussion_r115650129
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -306,6 +310,19 @@ public SendResult sendMessage(//
final SendMessageContext context, // 11
final DefaultMQProducerImpl producer // 12
) throws RemotingException, MQBrokerException, InterruptedException {
+
+ byte[] msgBody = msg.getBody();
+ Integer sysFlag = requestHeader.getSysFlag();
+ if (null != sysFlag && ((sysFlag & MessageSysFlag.COMPRESSED_FLAG) == MessageSysFlag.COMPRESSED_FLAG)) {
+ try {
+ msgBody = UtilAll.compress(msgBody, zipCompressLevel);
+ } catch (IOException e) {
+ requestHeader.setSysFlag(MessageSysFlag.clearCompressedFlag(sysFlag));
+ log.error("tryToCompressMessage exception", e);
--- End diff --
You mean why we log error on failing to compress message body?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #89: [ROCKETMQ-166] onException callback may captur...
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/89
@stevenschew Any idea?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---