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.

Sign up for GitHub

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

Jump to bottom

[17.0] [IMP] queue_job: HA job runner using session level advisory lock #673

Merged
merged 4 commits into from
Mar 17, 2025
Merged

[17.0] [IMP] queue_job: HA job runner using session level advisory lock #673

merged 4 commits into from
Mar 17, 2025

Conversation

qgroulard
Copy link
Contributor

Forward port of #668

@OCA-git-bot
Copy link
Contributor

Hi @guewen,
some modules you are maintaining are being modified, check this out!

LoisRForgeFlow
LoisRForgeFlow approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@LoisRForgeFlow
Copy link
Contributor

@guewen @sbidoul anything missing to get this one merged?

@simahawk
Copy link
Contributor

simahawk commented Dec 6, 2024 •
edited
Loading

It has been approved by @guewen here as well #668 yet @sbidoul had to provide feedback on odoo.sh if I'm not mistaken.

@sbidoul
Copy link
Member

sbidoul commented Dec 6, 2024

We deployed this on a 17.0 odoo.sh instance. It works and nobody yelled at us :)

Now this is not a high volume deployment.

I'm quite confident this PR is good, but we are going to deploy this on a couple of more intensive instances (not odoo.sh) to confirm. I would encourage others to to the same.

@sbidoul sbidoul mentioned this pull request Dec 6, 2024
Merged
@sbidoul sbidoul force-pushed the 17.0-forwardport-ha_runner-qgr branch from 918a966 to 992bc9b Compare December 6, 2024 08:44
@sbidoul
Copy link
Member

sbidoul commented Dec 6, 2024

Rebased

@LoisRForgeFlow
Copy link
Contributor

Hi @sbidoul

Still no issues with this improvement from our side. How is it going in your instances? Do you think we can merge?

sbidoul added 4 commits March 17, 2025 13:54
@sbidoul
[FIX] queue_job: close connection to databases without job queue
376c424 
Without this, we leak connections to Databases that don't have queue_job
installed.
@sbidoul
[FIX] queue_job: handle exceptions in Database constructor
e8247a1 
Without this we risk connection leaks in case of exceptions in the
constructor.
@sbidoul
[IMP] queue_job: HA job runner using session level advisory lock
f714651
@sbidoul
[IMP] queue_job: make sorting more explicit
d9f38cb
@sbidoul sbidoul force-pushed the 17.0-forwardport-ha_runner-qgr branch from 992bc9b to d9f38cb Compare March 17, 2025 12:54
@sbidoul
Copy link
Member

sbidoul commented Mar 17, 2025

Yes, let's merge. I have merged #748 and now this one.

Two great simplification in queue_job operations!

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 17.0-ocabot-merge-pr-673-by-sbidoul-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 400d8d3 into OCA:17.0 Mar 17, 2025
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at bd16ee3. Thanks a lot for contributing to OCA. ❤️

@sbidoul sbidoul deleted the 17.0-forwardport-ha_runner-qgr branch March 17, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guewen guewen guewen approved these changes

@gurneyalex gurneyalex gurneyalex approved these changes

@LoisRForgeFlow LoisRForgeFlow LoisRForgeFlow approved these changes

Assignees
No one assigned
Labels
approved merged 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@qgroulard @OCA-git-bot @LoisRForgeFlow @simahawk @sbidoul @guewen @gurneyalex