You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2015/04/23 04:56:28 UTC
[GitHub] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-6/pull/211
ACTIVEMQ6-94 Large Message over Bridge and server's transfer
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-6 new-Bridge
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-6/pull/211.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 #211
----
commit 86f386d984064b12e4d14d5906b87ba62132cf80
Author: Howard Gao <hg...@redhat.com>
Date: 2015-03-30T06:09:34Z
ACTIVEMQ6-94: HornetQ Bridge does not handle large messages
When sending a large message that exceeds the size of
Integer.MAX_VALUE, the bridge will get negative chunk size during
fowarding. And the resend cache is not limited so there is a
potential that it may get OutOfMemory exception.
commit 2984a10a24b511531ab4000baae298987cd973e9
Author: Clebert Suconic <cl...@apache.org>
Date: 2015-04-23T01:44:28Z
ACTIVEMQ6-94: Using proper flow control on very large messages over the bridge
This will remove some of the verifications written by Howard on his commit.
I did this to simplify the flow control
This closes #197
----
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95464278
give me some time to test it. I remember this producer window probably won't fix it.
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95404561
@gaohoward can you take a look on my changes please?
This will close your PR.. maybe you should close #197 already...
I would appreciate having at least 2 people looking into this PR (Martyn/Andy / @jbertram ?)
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95559754
Ok although the unit test pass, but with manual test I sometimes got OOM still. Look at this code in ChannelImpl.send():
resendCache.add(packet);
https://github.com/clebertsuconic/activemq-6/blob/new-Bridge/activemq-core-client/src/main/java/org/apache/activemq/core/protocol/core/impl/ChannelImpl.java#L294
When sending a large, each packet holds a chunk of it, and because it's stored in the resendCache, it doesn't get gc'ed after the bytes are sent to the other node. Eventually before the large send complete, all previous sent chunks will be held in memory.
any comment?
Howard
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95560165
I will take a look...
But the only thing supposed to wait on the channel is flow control.... that previous fix is not valid (it would cause serious performance issues on regular cases).
Besides 50M minLargeMessageSize is an invalid test.
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95560474
Ah, in the test setting 50M is just to make the test run faster. consider it's 3G large message...
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-6/pull/211
---
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] activemq-6 pull request: ACTIVEMQ6-94 Large Message over Bridge an...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-6/pull/211#issuecomment-95690679
@gaohoward if you are sending large messages this huge you will have to set the new setting on producerWindowSize through the bridge. that's the only thing we should support.
With a 50M min largeMessagesize you will be allocating huge buffers over the Native Layer as well.. it's bound to fail.
I have run your test with 3G files and it's working fine. So, this should be sufficient to fix this issue on master. I really don't want any layer to wait on resendCache to exaust.. that will cause huge delays on average sized large messages (such as 10M, 1M.. since you would require a round trip network for every chunk sent).
I think we are fine with this fix 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.
---