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

[IMP] connector-runner: graceful stop mechanism #69

Merged
merged 1 commit into from Jun 9, 2015

Conversation

Projects
None yet
4 participants
@sbidoul
Member

sbidoul commented May 16, 2015

The previous version was relying a daemon thread, which was ok, except it would continue to enqueue jobs while the server was stopping, and such jobs would never start.

This in combination with odoo/odoo#6738 should ensure jobs are left in a consistent state upon normal shutdown. Only a hard crash would leave jobs in a started state in which case those must be requeued manually.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 16, 2015

Coverage Status

Coverage decreased (-0.37%) to 74.21% when pulling 2ce1d62 on acsone:8.0-runnerstop-sbi into aa5cf84 on OCA:8.0.

coveralls commented May 16, 2015

Coverage Status

Coverage decreased (-0.37%) to 74.21% when pulling 2ce1d62 on acsone:8.0-runnerstop-sbi into aa5cf84 on OCA:8.0.

[IMP] connector-runner: graceful stop mechanism
The previous version was using a daemon thread, which as more or less
ok, except it could enqueue jobs while the server was stopping, and such
jobs would never start.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls May 16, 2015

Coverage Status

Coverage decreased (-0.41%) to 74.17% when pulling ce882d0 on acsone:8.0-runnerstop-sbi into aa5cf84 on OCA:8.0.

coveralls commented May 16, 2015

Coverage Status

Coverage decreased (-0.41%) to 74.17% when pulling ce882d0 on acsone:8.0-runnerstop-sbi into aa5cf84 on OCA:8.0.

@guewen

This comment has been minimized.

Show comment
Hide comment
@guewen

guewen May 20, 2015

Member

Very well observed! I confirm that started jobs are no longer interrupted with this patch and odoo/odoo#6738.

2015-05-20 11:44:16,198 14980 DEBUG openerp_dev openerp.addons.connector.controllers.main: <Job 341d9f10-09b4-40c7-9565-c17a09a8d1c0, priority:10> done
2015-05-20 11:44:16,202 14980 INFO openerp_dev openerp.service.server: Worker (14980) exiting. request_count: 8, registry count: 1.

Great! Just waiting on an answer on odoo/odoo#6738 now...

Member

guewen commented May 20, 2015

Very well observed! I confirm that started jobs are no longer interrupted with this patch and odoo/odoo#6738.

2015-05-20 11:44:16,198 14980 DEBUG openerp_dev openerp.addons.connector.controllers.main: <Job 341d9f10-09b4-40c7-9565-c17a09a8d1c0, priority:10> done
2015-05-20 11:44:16,202 14980 INFO openerp_dev openerp.service.server: Worker (14980) exiting. request_count: 8, registry count: 1.

Great! Just waiting on an answer on odoo/odoo#6738 now...

@lmignon

This comment has been minimized.

Show comment
Hide comment
@lmignon

lmignon Jun 2, 2015

Contributor

👍

Contributor

lmignon commented Jun 2, 2015

👍

@guewen

This comment has been minimized.

Show comment
Hide comment
@guewen

guewen Jun 8, 2015

Member

@lmignon did you backported this PR in v7 too?

Member

guewen commented Jun 8, 2015

@lmignon did you backported this PR in v7 too?

@lmignon

This comment has been minimized.

Show comment
Hide comment
@lmignon

lmignon Jun 8, 2015

Contributor

@guewen not yet. it will be done tomorrow as I need it

Contributor

lmignon commented Jun 8, 2015

@guewen not yet. it will be done tomorrow as I need it

@sbidoul

This comment has been minimized.

Show comment
Hide comment
@sbidoul

sbidoul Jun 8, 2015

Member

In v7 the runner is a separate process? In which case it should not be necessary to backport this: killing the runner before OpenERP should be sufficient.

Member

sbidoul commented Jun 8, 2015

In v7 the runner is a separate process? In which case it should not be necessary to backport this: killing the runner before OpenERP should be sufficient.

@guewen

This comment has been minimized.

Show comment
Hide comment
@guewen

guewen Jun 8, 2015

Member

In v7 the runner is a separate process? In which case it should not be necessary to backport this: killing the runner before OpenERP should be sufficient.

It makes sense.

@sbidoul If I understand correctly, we can already merge this one even if odoo/odoo#6738 is not merged yet. It won't prevent jobs to be interrupted, but it will prevent to push new jobs when exiting, is it correct?

Member

guewen commented Jun 8, 2015

In v7 the runner is a separate process? In which case it should not be necessary to backport this: killing the runner before OpenERP should be sufficient.

It makes sense.

@sbidoul If I understand correctly, we can already merge this one even if odoo/odoo#6738 is not merged yet. It won't prevent jobs to be interrupted, but it will prevent to push new jobs when exiting, is it correct?

@lmignon

This comment has been minimized.

Show comment
Hide comment
@lmignon

lmignon Jun 9, 2015

Contributor

@sbidoul you're right. The one I need to back port is odoo/odoo#6738

Contributor

lmignon commented Jun 9, 2015

@sbidoul you're right. The one I need to back port is odoo/odoo#6738

@sbidoul

This comment has been minimized.

Show comment
Hide comment
@sbidoul

sbidoul Jun 9, 2015

Member

@sbidoul https://github.com/sbidoul If I understand correctly, we can
already merge this one even if odoo/odoo#6738
odoo/odoo#6738 is not merged yet. It won't
prevent jobs to be interrupted, but it will prevent to push new jobs when
exiting, is it correct?

Correct.

Member

sbidoul commented Jun 9, 2015

@sbidoul https://github.com/sbidoul If I understand correctly, we can
already merge this one even if odoo/odoo#6738
odoo/odoo#6738 is not merged yet. It won't
prevent jobs to be interrupted, but it will prevent to push new jobs when
exiting, is it correct?

Correct.

@guewen

This comment has been minimized.

Show comment
Hide comment
@guewen

guewen Jun 9, 2015

Member

👍
Thanks!

Member

guewen commented Jun 9, 2015

👍
Thanks!

guewen added a commit that referenced this pull request Jun 9, 2015

Merge pull request #69 from acsone/8.0-runnerstop-sbi
[IMP] connector-runner: graceful stop mechanism

@guewen guewen merged commit cbf270f into OCA:8.0 Jun 9, 2015

2 checks passed

ci/runbot runbot build 2946882-69-ce882d (runtime 55s)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sbidoul sbidoul deleted the acsone:8.0-runnerstop-sbi branch Jun 9, 2015

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