Skip to content

[5.6] database queue transaction wrap#22433

Merged
taylorotwell merged 1 commit into
laravel:masterfrom
ph4r05:5.6_database_queue_tsx_wrap
Dec 14, 2017
Merged

[5.6] database queue transaction wrap#22433
taylorotwell merged 1 commit into
laravel:masterfrom
ph4r05:5.6_database_queue_tsx_wrap

Conversation

@ph4r05

@ph4r05 ph4r05 commented Dec 14, 2017

Copy link
Copy Markdown
Contributor

As discussed in #22394 with @sisve and @deleugpn the transaction in DatabaseQueue::pop is wrapped in the transaction closure for better readability and simplicity.

This also enables to remove a side effect of DatabaseQueue::marshalJob committing the transaction which is nonintuitive and confusing - potential breaking change for third-party code - the reason for adding to 5.6.

@ph4r05 ph4r05 force-pushed the 5.6_database_queue_tsx_wrap branch from 8641b84 to 4bf79be Compare December 14, 2017 20:30
- pop() transaction wrapped in closure for easier readability, removing side effect of marshalJob() which nonintuitively commits the transaction
@ph4r05 ph4r05 force-pushed the 5.6_database_queue_tsx_wrap branch from 4bf79be to c38ee67 Compare December 14, 2017 20:31
@taylorotwell

Copy link
Copy Markdown
Member

I have a somewhat strong suspicion we are breaking things and making deadlock issues worse here but will give it a shot. 😄

@taylorotwell taylorotwell merged commit c38ee67 into laravel:master Dec 14, 2017
@deleugpn

Copy link
Copy Markdown
Contributor

Looks great.

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

Successfully merging this pull request may close these issues.

3 participants