Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider implementing Driver::rejectMessage() #134

Open
jmikola opened this issue Dec 19, 2014 · 7 comments
Open

Consider implementing Driver::rejectMessage() #134

jmikola opened this issue Dec 19, 2014 · 7 comments

Comments

@jmikola
Copy link
Contributor

jmikola commented Dec 19, 2014

See conversation in #133 (comment) for context.

When drivers pop a message off the queue, they may leave it persisted but flip the visible flag so it's not picked up by future pops. In turn, acknowledgeMessage() may permanently remove the message from the queue. If it's relevant to enough drivers, rejectMessage() would revert the visible flag on messages, thereby allowing it to be picked up again.

Alternatively, you may decided that popped messages should simply be enqueued again if the work was not successfully completed. I don't really have an opinion on this, but wanted to create the issue as a placeholder for future discussion.

@sagikazarmark
Copy link
Contributor

Dup of #97 ?

@holtkamp
Copy link
Contributor

holtkamp commented Mar 1, 2017

I was about to create an issue for this, but might be better to keep it grouped:

"The DoctrineDriver uses a boolean field visible to mark whether a message should be fetched from a queue. When it is fetched, the field visible will be set to 0, indicating it is 'being processed' / in flight, after which it will be 'acknowledged' and thereby removed from the queue.

However, suppose something goes wrong during message consumption / the message is skipped for whatever reason and therefore not acknowledged (removed from queue), then it will remain on the queue invisible until the end of time.

A way to deal with this might be to change the visible flag to a timestamp invisible_until_timestamp. Amazon SQS defaults to in-flight times of 30 seconds... but other intervals might be useful to be configurable."

@henrikbjorn
Copy link
Contributor

Sounds reasonable, a PR would be great.

@holtkamp
Copy link
Contributor

holtkamp commented Mar 1, 2017

a PR would be great.

ok, will try to give it a shot. Is backwards compatibility (with the 'visible' column) a must? Think that will complicate the PR a bit...

@henrikbjorn
Copy link
Contributor

not until 1.0 final is tagged.

@holtkamp
Copy link
Contributor

holtkamp commented Mar 3, 2017

FYI: it might take some time before I come up with a PR. For a work-related project we decided to use the SQS Driver, since Amazon AWS offers nice monitoring tools including alarms with CloudWatch. So work on DoctrineDriver has to be in spare time...

@GDmac
Copy link

GDmac commented Nov 30, 2018

When setting up a consumer for a queue (from examples), a producer is instantiated
and with the FailureSubscriber added to the dispatcher.
Instead, since we're setting up a specific consumer anyway, one could add a custom listener that acknowledges the queue on the message and retries the rejected message a couple of times before failing.

// $eventDispatcher->addSubscriber(new EventListener\FailureSubscriber($producer));

$eventDispatcher->addListener('bernard.reject', function($event) use ($producer) {
  $envelope = $event->getEnvelope();
  $queue = $event->getQueue();  
  $queue->acknowledge($envelope);

  $message = $envelope->getMessage();
  $name = $message->getName();
  $arguments = $message->all();
  $arguments['RetryCount'] = (int) $message->get('RetryCount') + 1;
  $retrymessage = new PlainMessage($name, $arguments);

  if ($arguments['RetryCount'] >= 3) {
    $producer->produce($retrymessage, 'failed');
  } else {
    $producer->produce($retrymessage, 'send-newsletter');
  }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants