Implement a queue mechanism

Drafted first list of codes to handle

dave.mentens → made their first commit to this issue’s fork.

Also a patch for version 1.0.x

Added missing dependencies for 1.0.x patch

The issue fork is 10 commits behind, 20 commits ahead of the upstream repository. When I created the MR, it created it against the 1.0.x branch, not the 2.0.x branch.

The GraphService class should have a different name, as none of the classes that implement a service contains Service in their names.

I think it is also better to catch all exceptions thrown, because we now sometimes see a '503 Service Unavailable'.

The MR is also doing changes that should be better split on different issues, as they are not strictly necessary to implement a queue mechanism, for example adding the schema for the configuration file.

cgoffin → closed merge request !9

I tried to refactor, but also messed up some branches. My apologies. I created the new branch 3390319-add_queue_mechanism which started from the 1.0.x version. Here also a patch file to use in your projects.

Fixed a bug in de code changes.

apaderno → closed merge request !8

Found another bug with missing use statements.

It is better to postpone this, as the changes introduced from the linked issue are slightly different, even if the module will still implement a service class.

I will handle this in the next 6 hours.

The issue I see is that the queue we require needs to be strictly first in, last out; it should also allow us to always handle items respecting the Retry-After value returned from the Microsoft Graph API. This is not possible with the queue API, which does not even guarantee the default queue is delayable.

first in, last out

Hmm I'm not following here. Why would that be needed? Seems more logical to me that the oldest item get's processed first.

Retry-After value returned from the Microsoft Graph API. This is not possible with the queue API, which does not even guarantee the default queue is delayable.

I don't really see an issue here. I guess we could set the delay exception to the retry-after value. But in anyway, the queue is depending on how often you're running cron, and thus processing queue's.

We're running on Acquia where the max cron interval is 5 minutes, so we know it is possible there might be a small delay in the emails being sent. However the trade off of having the email sent VS it being lost forever because there was an exception is for us a major reason to have the queue. We've been running this for a week or so now in production and everything is working fine actually.

The issue is that the queue API does not guarantee the implemented queue is reliable (the order of items is preserved and every item will be executed at least once) nor delayable (an item can be released on a delay) which are the requisites for the queue the module should use.

Furthermore, for the queue API, keeping an item in the queue is an exception, not the rule. The module needs also to get from the queue only those items for which the Retry-After delay is already passed.

I did not say the module will not use any queue, but that queue cannot be the queue implemented by the queue API.

We've been running this for a week or so now in production and everything is working fine actually.

It works fine only if the site is using the DatabaseQueue class for queues, but the default queue class can be changed by modules or from site settings, and the used class is not required to implement DelayableQueueInterface nor ReliableQueueInterface.

Ah I see, we're not using DelayedRequeueException here

By default, cron tasks use the queue.database service, which implements a reliable and delayable queue.
If the site setting select a different queue, which is neither reliable nor delayable, that is an issue with the site settings, not the module. (As far as I can see, modules do not have a way to select a different queue implementation to pass to a QueueWorker plugin.)

There are still issues with the current patch:

• The correct way to delay an item is to throw a DelayedRequeueException exception
• The code checking for the Retry-Delay value does not verify the thrown exception type ($delay = ($e->getResponse() && $e->getResponse()->hasHeader('Retry-After')) ? (int) $e->getResponse()->getHeader('Retry-After')[0] : 600;)

For the first point, the only way to delay an item is to store in the item itself the delay the Microsoft Graph API required when throwing the exception. When the queue worker handles the item, it must first check the delay and be sure that delay is already passed, before handling the item.

Now the code just needs to back-ported to the 1.0.x branch.

Automatically closed - issue fixed for 2 weeks with no activity.

apaderno → closed merge request !11