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

Jobs in chains ignore ShouldBeUnique #49263

Closed
tricki opened this issue Dec 6, 2023 · 5 comments
Closed

Jobs in chains ignore ShouldBeUnique #49263

tricki opened this issue Dec 6, 2023 · 5 comments

Comments

@tricki
Copy link
Contributor

tricki commented Dec 6, 2023

Laravel Version

10.34.2

PHP Version

8.1.26

Database Driver & Version

mariadb 10.5.8 (Laravel Sail)

Description

When dispatching jobs as part of a chain (using Bus::chain()) the uniqueness is not guaranteed.

Steps To Reproduce

  1. Create a new job
  2. Implement the Illuminate\Contracts\Queue\ShouldBeUnique interface
  3. Dispatch the job as part of a chain multiple times
  4. Check that job is in the queue multiple times instead of just once

Dispatch example:

Bus::chain([
    new NewUniqueJob(),
])->dispatch();

Some details about why I could use this right now (also in case someone can suggest a better way): I have an application that acts as a proxy between two other systems (A and B). System A sends an update request for one, multiple or all entities (e.g. entity 1 has been updated). The application then fetches those changes from System A and saves them to the database. Then the application sends a general "change" request to System B, which will then load and import all entities from the application.

Here's a simple sequence diagram:
image

In the end I have three jobs that have to be run in sequence:

  1. Load the new data from System A and save to database
  2. Clean up database (e.g. removing entities which were deleted in System A)
  3. Trigger import in System B.

Additional notes:

  • The reason for this complex system is the rigidity of the other systems: Both their APIs are built using no-code tools and are therefore quite inflexible. Both in their data structure and extensibility.
  • They're separate jobs because the first one could be scoped to one, multiple or all entities, whereas the third one is always the same.
  • In the third job I'm actually using ShouldBeUniqueUntilProcessing, but since that interface extends ShouldQueue I assume that's not relevant here.
  • Previously those steps were done synchronously, because response time wasn't a concern, since it's just APIs. The main reason I refactored to using Jobs is to take advantage of the uniqueness feature.

I'm hoping this will be implemented. If not, I suggest explicitly mentioning that in the docs.

Copy link

github-actions bot commented Dec 7, 2023

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@tntsoft
Copy link

tntsoft commented Dec 9, 2023

I've opened #49294 where I've created a test in order to replicate this, but apparently it works as expected.

What I've done is I've created a unique job through the ShouldBeUnique interface and dispatched it through a chain 4 times, but it comes out only 1 time.

@tricki If you have time to provide a clean repository replicating this issue, that would help a ton. Or maybe take a look at my PR and let me know what I can do in order to replicate this.

@tricki
Copy link
Contributor Author

tricki commented Dec 13, 2023

Thanks for the response and writing the test! I'll definitely look into it again more closely and report back.

I have to say I never really used the queue before, so I'll have to familiarize myself with how they work, especially how the atomic locks work.

@driesvints
Copy link
Member

Hi all, is this still being worked on? Seems the test proves this works so this can be closed?

@driesvints
Copy link
Member

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel to open up a new issue if you're still experiencing this.

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

No branches or pull requests

4 participants