[DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

nigro_franz
HI folks,

I'm thinking how to improve (the performance and stability) of AMQP paging
and I've fallen into this behaviour for paging AMQ standard messages:
https://github.com/apache/activemq-artemis/blob/d5104656f9060d347b326c59e136dac0b3c404e0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java#L840


This reencode() is a very intensive operation and is being triggered on
depaging (exactly here
https://github.com/apache/activemq-artemis/blob/51c2022f388a5c70adbc5de6b799f9072960c2f1/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java#L154).
It force non durable AMQP messages to see their AMQP header to be created,
if not present, just to set that property: given that the header part of
AMQP messages is encoded at the beginning of the message, it forces a
reencode of the whole message.
I suppose this could be improved in different ways, but I was thinking to
always preset an header with durable == false in ((when appropriate, with
non-durable messages): it would make every non durable messages a lil
bigger, but will save lot of memory and CPU time on depaging, because the
reencode could be optimized and be partial (reencoding only the few bits
necessary on the header, now present).
I'm opened to discuss this and find together a smart way to improve
depaging: given that it happens when the JVM memory is supposed to be
precious I believe that saving time/memory there is crucial not just for
performance but for the broker stability.

Cheers,
Franz



I've already opened&solved
https://issues.apache.org/jira/browse/ARTEMIS-2661 to improve AMQP journal
loading by saving scanning AMQP message data if not strictly necessary and
it already deliver exceptional result: I would like to do the same on
paging, given that this really seems a low hanging fruit with an high
potential of improvement.



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

nigro_franz
Another (additional?) strategy could be to just add the (newEncodedHeaderSize
- oldEncodedHeaderSize) to the others positions and sizes if present (eg
deliveryAnnotationsPosition, encodedDeliveryAnnotationsSize, etc etc),
encoding any new header (or overwriting the original one, without allocating
a new data) and saving from scanning the message data again.
I would attempt a PR, but need some help from any AMQP guy (nudge nudge)
around here :P




--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

tabish121@gmail.com
In reply to this post by nigro_franz
On 3/19/20 9:43 AM, nigro_franz wrote:

> HI folks,
>
> I'm thinking how to improve (the performance and stability) of AMQP paging
> and I've fallen into this behaviour for paging AMQ standard messages:
> https://github.com/apache/activemq-artemis/blob/d5104656f9060d347b326c59e136dac0b3c404e0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java#L840
>
>
> This reencode() is a very intensive operation and is being triggered on
> depaging (exactly here
> https://github.com/apache/activemq-artemis/blob/51c2022f388a5c70adbc5de6b799f9072960c2f1/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java#L154).
> It force non durable AMQP messages to see their AMQP header to be created,
> if not present, just to set that property: given that the header part of
> AMQP messages is encoded at the beginning of the message, it forces a
> reencode of the whole message.
> I suppose this could be improved in different ways, but I was thinking to
> always preset an header with durable == false in ((when appropriate, with
> non-durable messages): it would make every non durable messages a lil
> bigger, but will save lot of memory and CPU time on depaging, because the
> reencode could be optimized and be partial (reencoding only the few bits
> necessary on the header, now present).

If the broker is going to update the message as it comes out of the
journal to indicate it has become durable then why not beat that to the
punch by just putting it in there that way when there no Header or the
existing Header is marked as non-durable?

* In the case of not having a header you'd then only need to first write
one and then the remaining already encoded message portion.

* In the case that there was a Header I think the AMQPMessage tracks
those location so you could update and re-encode only the Header and
then do as above and just write out the remainder of the previously
encoded message.

I wouldn't just add a Header to every message though as the broker
really should just relay the message as is if it didn't need to store it
in the first place.  I've always been a bit dubious on the whole marking
it durable just because it was paged thing but that might just be me.

> I'm opened to discuss this and find together a smart way to improve
> depaging: given that it happens when the JVM memory is supposed to be
> precious I believe that saving time/memory there is crucial not just for
> performance but for the broker stability.
>
> Cheers,
> Franz
>
>
>
> I've already opened&solved
> https://issues.apache.org/jira/browse/ARTEMIS-2661 to improve AMQP journal
> loading by saving scanning AMQP message data if not strictly necessary and
> it already deliver exceptional result: I would like to do the same on
> paging, given that this really seems a low hanging fruit with an high
> potential of improvement.
>
>
>
> --
> Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html


--
Tim Bish

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

clebertsuconic
Can we stop doing that check?

If we are out of memory, we could certainly write non durable messages
to disk, but that doesn't mean they became durable.

On Thu, Mar 19, 2020 at 12:21 PM Timothy Bish <[hidden email]> wrote:

>
> On 3/19/20 9:43 AM, nigro_franz wrote:
> > HI folks,
> >
> > I'm thinking how to improve (the performance and stability) of AMQP paging
> > and I've fallen into this behaviour for paging AMQ standard messages:
> > https://github.com/apache/activemq-artemis/blob/d5104656f9060d347b326c59e136dac0b3c404e0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java#L840
> >
> >
> > This reencode() is a very intensive operation and is being triggered on
> > depaging (exactly here
> > https://github.com/apache/activemq-artemis/blob/51c2022f388a5c70adbc5de6b799f9072960c2f1/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java#L154).
> > It force non durable AMQP messages to see their AMQP header to be created,
> > if not present, just to set that property: given that the header part of
> > AMQP messages is encoded at the beginning of the message, it forces a
> > reencode of the whole message.
> > I suppose this could be improved in different ways, but I was thinking to
> > always preset an header with durable == false in ((when appropriate, with
> > non-durable messages): it would make every non durable messages a lil
> > bigger, but will save lot of memory and CPU time on depaging, because the
> > reencode could be optimized and be partial (reencoding only the few bits
> > necessary on the header, now present).
>
> If the broker is going to update the message as it comes out of the
> journal to indicate it has become durable then why not beat that to the
> punch by just putting it in there that way when there no Header or the
> existing Header is marked as non-durable?
>
> * In the case of not having a header you'd then only need to first write
> one and then the remaining already encoded message portion.
>
> * In the case that there was a Header I think the AMQPMessage tracks
> those location so you could update and re-encode only the Header and
> then do as above and just write out the remainder of the previously
> encoded message.
>
> I wouldn't just add a Header to every message though as the broker
> really should just relay the message as is if it didn't need to store it
> in the first place.  I've always been a bit dubious on the whole marking
> it durable just because it was paged thing but that might just be me.
>
> > I'm opened to discuss this and find together a smart way to improve
> > depaging: given that it happens when the JVM memory is supposed to be
> > precious I believe that saving time/memory there is crucial not just for
> > performance but for the broker stability.
> >
> > Cheers,
> > Franz
> >
> >
> >
> > I've already opened&solved
> > https://issues.apache.org/jira/browse/ARTEMIS-2661 to improve AMQP journal
> > loading by saving scanning AMQP message data if not strictly necessary and
> > it already deliver exceptional result: I would like to do the same on
> > paging, given that this really seems a low hanging fruit with an high
> > potential of improvement.
> >
> >
> >
> > --
> > Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
>
>
> --
> Tim Bish
>


--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

tabish121@gmail.com
On 3/19/20 2:33 PM, Clebert Suconic wrote:
> Can we stop doing that check?
>
> If we are out of memory, we could certainly write non durable messages
> to disk, but that doesn't mean they became durable.

That has always been my thought in this, it is not suddenly durable
simply because it was paged.


>
> On Thu, Mar 19, 2020 at 12:21 PM Timothy Bish <[hidden email]> wrote:
>> On 3/19/20 9:43 AM, nigro_franz wrote:
>>> HI folks,
>>>
>>> I'm thinking how to improve (the performance and stability) of AMQP paging
>>> and I've fallen into this behaviour for paging AMQ standard messages:
>>> https://github.com/apache/activemq-artemis/blob/d5104656f9060d347b326c59e136dac0b3c404e0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java#L840
>>>
>>>
>>> This reencode() is a very intensive operation and is being triggered on
>>> depaging (exactly here
>>> https://github.com/apache/activemq-artemis/blob/51c2022f388a5c70adbc5de6b799f9072960c2f1/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java#L154).
>>> It force non durable AMQP messages to see their AMQP header to be created,
>>> if not present, just to set that property: given that the header part of
>>> AMQP messages is encoded at the beginning of the message, it forces a
>>> reencode of the whole message.
>>> I suppose this could be improved in different ways, but I was thinking to
>>> always preset an header with durable == false in ((when appropriate, with
>>> non-durable messages): it would make every non durable messages a lil
>>> bigger, but will save lot of memory and CPU time on depaging, because the
>>> reencode could be optimized and be partial (reencoding only the few bits
>>> necessary on the header, now present).
>> If the broker is going to update the message as it comes out of the
>> journal to indicate it has become durable then why not beat that to the
>> punch by just putting it in there that way when there no Header or the
>> existing Header is marked as non-durable?
>>
>> * In the case of not having a header you'd then only need to first write
>> one and then the remaining already encoded message portion.
>>
>> * In the case that there was a Header I think the AMQPMessage tracks
>> those location so you could update and re-encode only the Header and
>> then do as above and just write out the remainder of the previously
>> encoded message.
>>
>> I wouldn't just add a Header to every message though as the broker
>> really should just relay the message as is if it didn't need to store it
>> in the first place.  I've always been a bit dubious on the whole marking
>> it durable just because it was paged thing but that might just be me.
>>
>>> I'm opened to discuss this and find together a smart way to improve
>>> depaging: given that it happens when the JVM memory is supposed to be
>>> precious I believe that saving time/memory there is crucial not just for
>>> performance but for the broker stability.
>>>
>>> Cheers,
>>> Franz
>>>
>>>
>>>
>>> I've already opened&solved
>>> https://issues.apache.org/jira/browse/ARTEMIS-2661 to improve AMQP journal
>>> loading by saving scanning AMQP message data if not strictly necessary and
>>> it already deliver exceptional result: I would like to do the same on
>>> paging, given that this really seems a low hanging fruit with an high
>>> potential of improvement.
>>>
>>>
>>>
>>> --
>>> Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
>>
>> --
>> Tim Bish
>>
>

--
Tim Bish

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

nigro_franz
Maybe we were doing it due to DLA (just guessing) but what you've said
makes lot of sense to me...

Il gio 19 mar 2020, 19:35 Timothy Bish <[hidden email]> ha scritto:

> On 3/19/20 2:33 PM, Clebert Suconic wrote:
> > Can we stop doing that check?
> >
> > If we are out of memory, we could certainly write non durable messages
> > to disk, but that doesn't mean they became durable.
>
> That has always been my thought in this, it is not suddenly durable
> simply because it was paged.
>
>
> >
> > On Thu, Mar 19, 2020 at 12:21 PM Timothy Bish <[hidden email]>
> wrote:
> >> On 3/19/20 9:43 AM, nigro_franz wrote:
> >>> HI folks,
> >>>
> >>> I'm thinking how to improve (the performance and stability) of AMQP
> paging
> >>> and I've fallen into this behaviour for paging AMQ standard messages:
> >>>
> https://github.com/apache/activemq-artemis/blob/d5104656f9060d347b326c59e136dac0b3c404e0/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java#L840
> >>>
> >>>
> >>> This reencode() is a very intensive operation and is being triggered on
> >>> depaging (exactly here
> >>>
> https://github.com/apache/activemq-artemis/blob/51c2022f388a5c70adbc5de6b799f9072960c2f1/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/PagedReferenceImpl.java#L154
> ).
> >>> It force non durable AMQP messages to see their AMQP header to be
> created,
> >>> if not present, just to set that property: given that the header part
> of
> >>> AMQP messages is encoded at the beginning of the message, it forces a
> >>> reencode of the whole message.
> >>> I suppose this could be improved in different ways, but I was thinking
> to
> >>> always preset an header with durable == false in ((when appropriate,
> with
> >>> non-durable messages): it would make every non durable messages a lil
> >>> bigger, but will save lot of memory and CPU time on depaging, because
> the
> >>> reencode could be optimized and be partial (reencoding only the few
> bits
> >>> necessary on the header, now present).
> >> If the broker is going to update the message as it comes out of the
> >> journal to indicate it has become durable then why not beat that to the
> >> punch by just putting it in there that way when there no Header or the
> >> existing Header is marked as non-durable?
> >>
> >> * In the case of not having a header you'd then only need to first write
> >> one and then the remaining already encoded message portion.
> >>
> >> * In the case that there was a Header I think the AMQPMessage tracks
> >> those location so you could update and re-encode only the Header and
> >> then do as above and just write out the remainder of the previously
> >> encoded message.
> >>
> >> I wouldn't just add a Header to every message though as the broker
> >> really should just relay the message as is if it didn't need to store it
> >> in the first place.  I've always been a bit dubious on the whole marking
> >> it durable just because it was paged thing but that might just be me.
> >>
> >>> I'm opened to discuss this and find together a smart way to improve
> >>> depaging: given that it happens when the JVM memory is supposed to be
> >>> precious I believe that saving time/memory there is crucial not just
> for
> >>> performance but for the broker stability.
> >>>
> >>> Cheers,
> >>> Franz
> >>>
> >>>
> >>>
> >>> I've already opened&solved
> >>> https://issues.apache.org/jira/browse/ARTEMIS-2661 to improve AMQP
> journal
> >>> loading by saving scanning AMQP message data if not strictly necessary
> and
> >>> it already deliver exceptional result: I would like to do the same on
> >>> paging, given that this really seems a low hanging fruit with an high
> >>> potential of improvement.
> >>>
> >>>
> >>>
> >>> --
> >>> Sent from:
> http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
> >>
> >> --
> >> Tim Bish
> >>
> >
>
> --
> Tim Bish
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

nigro_franz
I've created a PR and the issue
https://issues.apache.org/jira/browse/ARTEMIS-2669 to address this: we've
chosen to remove that old code, simplifying the current implementation and
making the treatment of AMQP not durable messages more correct while
depaging. Thanks everybody for the discussion and help :)



--
Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSSION] AMQP paging is triggering reencode: it's possible to avoid it?

Robbie Gemmell
Removing that behaviour seems reasonable, a non-durable message isnt
really durable just because the broker decided to page it.

That said, it seems an odd thing to be doing deliberately without some
particular reason, i.e is removing it going to be undoing something or
reintroducing some other behaviour it had aimed to avoid, which
additional changes might also be needed to better address?

Robbie

On Sat, 21 Mar 2020 at 11:36, nigro_franz <[hidden email]> wrote:

>
> I've created a PR and the issue
> https://issues.apache.org/jira/browse/ARTEMIS-2669 to address this: we've
> chosen to remove that old code, simplifying the current implementation and
> making the treatment of AMQP not durable messages more correct while
> depaging. Thanks everybody for the discussion and help :)
>
>
>
> --
> Sent from: http://activemq.2283324.n4.nabble.com/ActiveMQ-Dev-f2368404.html