Fix retries on InMemory asynchronous queues #521
Conversation
| // The exception should cause the message to be retried | ||
| self::expectException(Exception::class); | ||
|
|
||
| $ecotoneTestSupport->run('async', ExecutionPollingMetadata::createWithTestingSetup()); |
There was a problem hiding this comment.
Is this the wanted behavior (silently dropping failing messages) ?
Hmm, that should happen only in case we do have strategy set for IGNORE (ignoring failing messages).
The default for in memory is resend, therefore in case of failure it will be simply send over again to the original channel without exception.
Are you sure Messages are lost? (This test will fail because no errors will be thrown, however Message should still be available in the Channel)
There was a problem hiding this comment.
Yes, for in memory queue channel message are lost. This is because of this line:
$pollingMetadata->isStoppedOnError() is true when testing
There was a problem hiding this comment.
This will happen due to ExecutionPollingMetadata::createWithTestingSetup() which sets up polling metadata to stop on error.
There was a problem hiding this comment.
Yes but I would expect the message to be sent back to the channel for retry. This was the behavior before.
Without sending back to the channel, you have different behavior with acknowledgeable channel and non acknowledgeable
There was a problem hiding this comment.
In case of stop, we don't either ack or nack those Messages in the Broker, as for some implementation like Kafka it would mean skipping over that Message. However for In Memory, it will mean that Message is lost, because it's removed the Queue Channel right away.
We would have to change the implementation of Queue Channel for receive, as currently it simply drop the Message from the Queue right away.
Meaning when it reaches the consumer stop, Message is already gone.
This means that InMemoryAcknowledgeCallback would have to drop that Message on accept/reject/resend, instead of doing that right away on receive.
Are you happy to adjust the implementation for this (but it may require some work)? I am happy to approve such change
Why is this change proposed?
Since Ecotone 1.260.0 and #499 , the behavior of in memory channel regarding retries has changed.
Here is a test case passing on 1.259.3 and failing since 1.260.0
Is this the wanted behavior (silently dropping failing messages) ?
Pull Request Contribution Terms