[
https://issues.apache.org/jira/browse/ARTEMIS-2617?focusedWorklogId=384134&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-384134
]
ASF GitHub Bot logged work on ARTEMIS-2617:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 09/Feb/20 20:14
Start Date: 09/Feb/20 20:14
Worklog Time Spent: 10m
Work Description: franz1981 commented on issue #2975: ARTEMIS-2617
Improve AMQP Journal loading
URL: https://github.com/apache/activemq-artemis/pull/2975#issuecomment-583875107
@tabish121 @gemmellr @clebertsuconic @michaelandrepearce
This PR contains many changes and I know there are parts that could be
simplified a lot.
The most important bits that can really be a game-changer for AMQP on
journal loading are on
https://github.com/apache/activemq-artemis/pull/2975/commits/fc77a546ca1ee047218e938f0413c55ed8c838c6.
So please review especially that part.
An important note:
I've found that `AMQPMessagePersisterV2::decode` does something weird re
`AMQPMessage::setAddress`: `AMQPMessagePersister::decode` can
`AMQPMessage::setAddress`, allocating `AMQPMessage::extraProperties`, but
`AMQPMessagePersisterV2::decode` can decode a new `extraProperties` totally
overwriting the existing one. Is it a bug or it's suppose to work like that?
I see that this behaviour has been introduced on `ARTEMIS-1858` in
1ae2784dc6075875b18780fa8ba40f86cb895f7b with this comment:
```java
/**
* This will set the address on CoreMessage.
*
* Note for AMQPMessages:
* in AMQPMessages this will not really change the address on the
message. Instead it will add a property
* on extraProperties which only transverse internally at the broker.
* Whatever you change here it won't affect anything towards the received
message.
*
* If you wish to change AMQPMessages address you will have to do it
directly at the AMQP Message, however beware
* that AMQPMessages are not supposed to be changed at the broker, so
only do it if you know what you are doing.
* @param address
* @return
*/
Message setAddress(SimpleString address);
```
I've tried to mitigate this by using `CoreMessageObjectPools` to save
`SimpleString` allocations
[here](https://github.com/franz1981/activemq-artemis/blob/918800b309009fd440bf83b69b54a0d4f49db940/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessagePersisterV2.java#L78)
and by allowing `TypedProperties::decode` to append/replace any existing
`extraProperty`, saving an `HashMap` allocation, but IMO it could be addressed
by properly fixing it, wdyt? any idea how?
----------------------------------------------------------------
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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 384134)
Time Spent: 1h (was: 50m)
> Improve AMQP Journal loading
> ----------------------------
>
> Key: ARTEMIS-2617
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2617
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Francesco Nigro
> Priority: Major
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Journal loading AMQP messages always force decoding message data thus
> creating garbage
> on broker startup.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
