Track the number of times a queue item is claimed and pass as an optional param to queue workers

Created on 8 April 2025, 14 days ago

Problem/Motivation

This is not exactly the cron or batch systems, but related. Not seeing a component for queue system

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Active

Version

11.1 πŸ”₯

Component

cron system

Created by

πŸ‡ΊπŸ‡ΈUnited States pwolanin

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @pwolanin
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    I'm started the work on the issue fork from πŸ“Œ BatchQueue::claimItem() can get stuck with huge data in batch queue Needs review since we need a similar change anyhow

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    pwolanin β†’ changed the visibility of the branch 3517969-track-the-number to hidden.

  • Merge request !11768Resolve #3517969 "Count claims" β†’ (Open) created by pwolanin
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    The proposed fix would also resolve ✨ Cron queue workers should receive queued item creation data Active

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin
  • Pipeline finished with Failed
    13 days ago
    Total: 525s
    #468474
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    I made a rough start on a new interface. Probably we should define a specific object class that is returned by a queue to represent an item so that non-database queue implementations can handle this however they want and the queue worker is ignorant of those details.

  • Pipeline finished with Failed
    13 days ago
    Total: 275s
    #468494
  • Pipeline finished with Failed
    13 days ago
    Total: 163s
    #468502
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    Thinking though this - it might be preferable if the new interface extends the old one so that the queue workers adopting the new interface are still required to be compatible with older code that's not calling the new method. They should still work with the data alone.

  • Pipeline finished with Failed
    7 days ago
    Total: 194s
    #473905
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin
  • Pipeline finished with Success
    7 days ago
    Total: 519s
    #473927
  • Pipeline finished with Failed
    6 days ago
    Total: 191s
    #474979
  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    Add a MockTime class per discussion with @longwave and

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    still needs an update hook

  • Pipeline finished with Failed
    6 days ago
    Total: 237s
    #475016
  • Pipeline finished with Failed
    6 days ago
    Total: 236s
    #475079
  • Pipeline finished with Canceled
    6 days ago
    Total: 125s
    #475098
  • Pipeline finished with Failed
    6 days ago
    Total: 175s
    #475103
  • Pipeline finished with Failed
    6 days ago
    Total: 426s
    #475118
  • Pipeline finished with Success
    6 days ago
    Total: 339s
    #475137
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Just want to plug Dead Letter Queue β†’ here, which solves this problem in contrib.

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    @dieterholvoet thanks - took a quick look. Yes, that seems similar, but it's taking a very specific action based on a fixed threshold for failures. The idea here is that core is not going to take any action or have a threshold, but this enables the queue worker to have that logic if desired.

    I do think it's interesting that the number of tries is decremented if the item is released in that module's code

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin
  • πŸ‡¨πŸ‡­Switzerland bircher πŸ‡¨πŸ‡Ώ

    I think this looks good.
    We deliberated that modules can support both versions by indirectly implementing the interface by extending the base class and then just add the new method which can then take advantage of this change. But as the documentation on the method signature states, it should also just work without the new object.

    This issue is not about deprecating processItem but maybe we can have a discussion about that in a followup issue.

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Do we really need a new interface and method here? Why not just add the metadata as an optional argument to the processItem function and add it to the interface in a new major?

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    @dieterholvoet I discussed quite a bit with bircher. The approach in the patch mean that you can have the same queue worker class be valid and work in both D10 and 11 as long as it extends the base class so this be just a small feature addition in 11 and not break anything.

    I'm not sure in terms of current core BC and deprecation policy if we'd ever just add the argument to the existing interface since that would break all the existing implementations. PHP doesn't allow you to add even an optional argument to the interface without it being also added in all the implementations.

    At some future point we could deprecate the processItem() method and just support the new method and interface?

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Why not postpone adding the argument to the interface until the next major version and until then make it an optional argument in worker classes like this:

    public function processItem($data, ?object $metadata = NULL): void

    calling the method like this:

    $worker->processItem($item->data, $metadata);

    That way it works both for workers with and without the method and the necessary changes to queue workers will be a lot more straightforward.

    At some future point we could deprecate the processItem() method and just support the new method and interface?

    If you're going to eventually do a breaking change, might as well stick to the same interface and method name and make the breaking change the addition of the new argument to the existing method, no?

  • πŸ‡ΊπŸ‡ΈUnited States pwolanin

    Why would I want to wait for Drupal 12 or later when we can solve the problem now? I'm proposing this because I need this kind of feature to solve problems with queue processing sooner than later. I'd rather see it in core now to benefit everyone.

    I understand it would be possible to follow the path outlined in the docs to add a new argument that would be an actual part of the interface in 12. I think this is what you are suggesting? https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... β†’

    I'dd defer to the commiters if they think that's the right choice.

Production build 0.71.5 2024