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

Dispatching multiple jobs within transaction (after_commit=true) leads to tags (for all jobs) of last pushed job #950

Closed
graemlourens opened this issue Jan 2, 2021 · 3 comments · Fixed by #951
Assignees
Labels

Comments

@graemlourens
Copy link
Contributor

graemlourens commented Jan 2, 2021

  • Horizon Version: 5.6.3
  • Laravel Version: 8.19.0
  • PHP Version: 7.4.12
  • Redis Driver & Version: phpredis 5.3.2

Description:

We are using the queue config option 'after_commit' set to true, in order that all jobs that are dispatched within transactions, are pushed after the transaction commits.

We use horizon tags, and saw lately that the tags did not match up with what the actual job.
After some digging we found out that when having after_commit set to true, the tags that are generated are taken from the previous job that was pushed to queue, not the actual one that i being pushed.

We followed this down the rabbits hole to RedisQueue.php in the method pushRaw. When after_commit is set to true, $this->lastPushed that is used to 'prepare' the job (extract tags etc) contains the last job that was pushed instead of the actual one that is being prepared.

Steps To Reproduce:

  1. Create 2 different jobs with each either manually tagging or passing a model as a constructor value
  2. Set after_commit to true in your queue config.
  3. Dispatch both jobs within a transaction:
        DB::transaction(function() {
            dispatch(new JobA());
            dispatch(new JobB());
        });

If you set a breakpoint in RedisQueue.php on the first line of pushRaw you will see that in both cases $this->lastPushed is equals to the JobB job instance, and therefore is passed along to the prepare method which will falsely determine the tags (and other things)

The tags that are determined (before being pushed to queue, and put into payload) will be those of JobB also for JobA.

I have tried to identify where this issue is coming from but can currently can not pinpoint it exactly nor propose a solution.

Hope this will make immediate sense to @themsaid

Kind regards, Graem

@driesvints
Copy link
Member

Thanks for the report @graemlourens. Mohamed has sent in a PR here: #951

@graemlourens
Copy link
Contributor Author

graemlourens commented Jan 4, 2021

Fantastic, thank you both @driesvints and @themsaid for the swift fixing! I'll check the fix out in a couple of hours to confirm our logs are showing the correct tags.

@graemlourens
Copy link
Contributor Author

I can confirm the fix works as intended. Tags are not correctly assigned to the correct jobs. Thx again!

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

Successfully merging a pull request may close this issue.

3 participants