You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by mtaylor <gi...@git.apache.org> on 2017/10/05 11:14:32 UTC
[GitHub] activemq-artemis pull request #1574: ARTEMIS-1444 Support Messages > Journal...
GitHub user mtaylor opened a pull request:
https://github.com/apache/activemq-artemis/pull/1574
ARTEMIS-1444 Support Messages > JournalBufferSize in all Protocols
Notice the use of a deprecated method message.getBodyBuffer() in ServerSessionImpl. It's valid usage in this case.
@clebertsuconic perhaps this should not be deprecated. The alternative (to use ICoreMessage interface) would require significant refactor.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mtaylor/activemq-artemis ARTEMIS-1444
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1574.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 #1574
----
commit 1e4cb1049174b7571da023c0d23f595f8a4b02fb
Author: Martyn Taylor <mt...@redhat.com>
Date: 2017-10-05T10:32:32Z
ARTEMIS-1444 Support Messages > JournalBufferSize in all Protocols
----
---
[GitHub] activemq-artemis pull request #1574: ARTEMIS-1444 Support Messages > Journal...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1574#discussion_r142941151
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -1309,12 +1309,30 @@ public RoutingStatus send(final Message message,
return send(getCurrentTransaction(), message, direct, noAutoCreateQueue);
}
+
+ private LargeServerMessage messageToLargeMessage(Message message) throws Exception {
+ LargeServerMessage lsm = getStorageManager().createLargeMessage(storageManager.generateID(), message);
--- End diff --
You need to change this to:
```java
ICoreMessage coreMessage = message.toCore();
LargeServerMessage lsm = getStorageManager().createLargeMessage(storageManager.generateID(), coreMessage);
coreMessage.getReadOnlyBodyBuffer()
```
Also:
- You are losing properties here.. you need to copy the messages from message into largeMessage.
- The createLargeMessage was intended for serverSend operations.. it will add a pending record on the journal.. you need to make sure the complete is called somehow.. Most likely if you restart the system with this message pending, it will be deleted since there will be a pending large message record on the journal/database.
I take this as incomplete.
---
[GitHub] activemq-artemis issue #1574: ARTEMIS-1444 Support Messages > JournalBufferS...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1574
I would first convert a message large to core. Using .toCore() then use getBufferReadOnly.
I can tweak the code for you if you like.
---
[GitHub] activemq-artemis pull request #1574: ARTEMIS-1444 Support Messages > Journal...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1574
---
[GitHub] activemq-artemis issue #1574: ARTEMIS-1444 Support Messages > JournalBufferS...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1574
@mtaylor did you test it?
The test you added is broken... I had to fix it with this:
```java
private LargeServerMessage messageToLargeMessage(Message message) throws Exception {
ICoreMessage coreMessage = message.toCore();
LargeServerMessage lsm = getStorageManager().createLargeMessage(storageManager.generateID(), coreMessage);
ActiveMQBuffer buffer = coreMessage.getReadOnlyBodyBuffer();
byte[] body = new byte[buffer.readableBytes()];
buffer.readBytes(body);
lsm.addBytes(body);
lsm.releaseResources();
lsm.putLongProperty(Message.HDR_LARGE_BODY_SIZE, body.length);
return lsm;
}
```
---
[GitHub] activemq-artemis issue #1574: ARTEMIS-1444 Support Messages > JournalBufferS...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1574
@clebertsuconic Tests were passing, but I blindly copied your suggestions ;) and didn't rerun. Change is fine with me. Cheers.
---
[GitHub] activemq-artemis issue #1574: ARTEMIS-1444 Support Messages > JournalBufferS...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1574
@mtaylor I am merging with my commit in top of yours.. feel free to add a new commit if you find a better way.
---