[ 
https://issues.apache.org/jira/browse/AMQ-5716?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michal Kubricht updated AMQ-5716:
---------------------------------
    Description: 
I might be wrong, but I think that there is a mistake in 
{{FilePendingMessageCursor}} in method {{isFull}}.
The logic implemented in code is:
 -- isFull returns true when any of "memory usage is full" or "temp usage is 
full".
>From my point of view this is a mistake and logic should be:
 -- isFull returns true only when both of "memory usage" and "temp usage" are 
full.

The reason why should this be fixed lies in the impact that this mistake caused 
us in our project:
TopicSubscription got into endless loop in method {{add(MessageReference)}} 
when sending message into topic and memory limit reached 100% (it passed 100% 
in our case, but this is complete different story). Following conditions were 
fulfilled:
 * the dispatched queue reached consumer prefetch size,
 * producer flow control was disabled,
 * and finally related pending message cursor was full (because of memory 
usage).
The temporary storage remained at 0%, it wasn't created at all.

I understand what is the loop good for, it waits until there is a space in 
pending message cursor. This is a correct behavior. Anyway, when topic uses 
{{FilePendingMessageCursor}}, it loops for no good reason, because the cursor 
itself can handle the situation when memory limit is reached/full. It dumps all 
messages from memory list into disk list into temporary storage.

I am providing a patch file for this fix and very basic JUnit test for it, if 
this is acknowledged as bug and its solution.
Third file is a JUnit test is not considered to be part of commit (it never 
ends without any fix provided), but shows as simple as possible the topic 
subscription stuck scenario.

If the cursor would return isFull as false, it would solve described situation. 
It is sure that I cannot know or even guess that this implementation of 
{{FilePendingMessageCursor}} is there for some other reason, so please take 
this only as my personal recommendation after some time spent on analysis of 
problem in our project.

  was:
I might be wrong, but I think that there is a mistake in 
{{FilePendingMessageCursor}} in method {{isFull}}.
The logic implemented in code is:
 -- isFull returns true when any of "memory usage is full" or "temp usage is 
full".
>From my point of view this is a mistake and logic should be:
 -- isFull returns true only when both of "memory usage" and "temp usage" are 
full.

The reason why should this be fixed lies in the impact that this mistake caused 
us in our project:
TopicSubscription got into endless loop in method {{add(MessageReference)}} 
when sending message into topic and memory limit reached 100% (it passed 100% 
in our case, but this is complete different story). Following conditions were 
fulfilled:
 * the dispatched queue reached consumer prefetch size,
 * producer flow control was disabled,
 * and finally related pending message cursor was full (because of memory 
usage).
The temporary storage remained at 0%, it wasn't created at all.

I understand what is the loop good for, it waits until there is a space in 
pending message cursor. This is a correct behavior. Anyway, when topic uses 
{{FilePendingMessageCursor}}, it loops for no good reason, because the cursor 
itself can handle the situation when memory limit is reached/full. It dumps all 
messages from memory list into disk list into temporary storage.

I am providing a patch file for this fix and very basic JUnit test for it, if 
this is acknowledged as bug and its solution.
Third file is a JUnit test is not considered to be part of commit, but shows as 
simple as possible the topic subscription stuck scenario.

If the cursor would return isFull as false, it would solve described situation. 
It is sure that I cannot know or even guess that this implementation of 
{{FilePendingMessageCursor}} is there for some other reason, so please take 
this only as my personal recommendation after some time spent on analysis of 
problem in our project.


> TopicSubscription gets into endless loop when memory limit reached
> ------------------------------------------------------------------
>
>                 Key: AMQ-5716
>                 URL: https://issues.apache.org/jira/browse/AMQ-5716
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 5.7.0
>         Environment: JDK-1.6.0_38
>            Reporter: Michal Kubricht
>
> I might be wrong, but I think that there is a mistake in 
> {{FilePendingMessageCursor}} in method {{isFull}}.
> The logic implemented in code is:
>  -- isFull returns true when any of "memory usage is full" or "temp usage is 
> full".
> From my point of view this is a mistake and logic should be:
>  -- isFull returns true only when both of "memory usage" and "temp usage" are 
> full.
> The reason why should this be fixed lies in the impact that this mistake 
> caused us in our project:
> TopicSubscription got into endless loop in method {{add(MessageReference)}} 
> when sending message into topic and memory limit reached 100% (it passed 100% 
> in our case, but this is complete different story). Following conditions were 
> fulfilled:
>  * the dispatched queue reached consumer prefetch size,
>  * producer flow control was disabled,
>  * and finally related pending message cursor was full (because of memory 
> usage).
> The temporary storage remained at 0%, it wasn't created at all.
> I understand what is the loop good for, it waits until there is a space in 
> pending message cursor. This is a correct behavior. Anyway, when topic uses 
> {{FilePendingMessageCursor}}, it loops for no good reason, because the cursor 
> itself can handle the situation when memory limit is reached/full. It dumps 
> all messages from memory list into disk list into temporary storage.
> I am providing a patch file for this fix and very basic JUnit test for it, if 
> this is acknowledged as bug and its solution.
> Third file is a JUnit test is not considered to be part of commit (it never 
> ends without any fix provided), but shows as simple as possible the topic 
> subscription stuck scenario.
> If the cursor would return isFull as false, it would solve described 
> situation. It is sure that I cannot know or even guess that this 
> implementation of {{FilePendingMessageCursor}} is there for some other 
> reason, so please take this only as my personal recommendation after some 
> time spent on analysis of problem in our project.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to