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

long running fetchmail blocks cron #7464

Open
yelizariev opened this Issue Jul 6, 2015 · 20 comments

Comments

Projects
None yet
7 participants
@yelizariev
Contributor

yelizariev commented Jul 6, 2015

Sometimes fetchmail run really long (10-20 hours):

2015-07-04 18:31:40,019 20702 DEBUG eg-prices.com openerp.addons.base.ir.ir_cron: 26491.446s (fetchmail.server, _fetch_mails)

2015-07-05 21:04:40,312 29476 DEBUG eg-prices.com openerp.addons.base.ir.ir_cron: 31459.958s (fetchmail.server, _fetch_mails)

2015-07-06 06:15:15,060 29476 DEBUG www.it-projects.info openerp.addons.base.ir.ir_cron: 63809.573s (fetchmail.server, _fetch_mails)

It completely blocks cron, to say nothing of fetching mails.

I think some timeout shoud be set, e.g. socket.setdefaulttimeout

@yelizariev

This comment has been minimized.

Show comment
Hide comment
@yelizariev

yelizariev Jul 6, 2015

Contributor

I use odoo 8.0

# git log -n 1
commit 0f01df42eaf662987abe5f069ca8df2e2caf8e37
Merge: 114e95b f15cbd6
Author: Olivier Dony <odo@openerp.com>
Date:   Thu Jun 12 18:59:15 2014 +0200

    [MERGE] Forward-port saas-5 up to f15cbd6
Contributor

yelizariev commented Jul 6, 2015

I use odoo 8.0

# git log -n 1
commit 0f01df42eaf662987abe5f069ca8df2e2caf8e37
Merge: 114e95b f15cbd6
Author: Olivier Dony <odo@openerp.com>
Date:   Thu Jun 12 18:59:15 2014 +0200

    [MERGE] Forward-port saas-5 up to f15cbd6
@Yenthe666

This comment has been minimized.

Show comment
Hide comment
@Yenthe666

Yenthe666 Jul 6, 2015

Contributor

I've experienced this problem on some Odoo's too. In fact this seems to be the case for multiple ir.cron tasks. Somehow every once in a while they seem to run stuck.
I honestly think that a time out would be a good fix. If it takes unusually long it should time-out and should then try again.

Contributor

Yenthe666 commented Jul 6, 2015

I've experienced this problem on some Odoo's too. In fact this seems to be the case for multiple ir.cron tasks. Somehow every once in a while they seem to run stuck.
I honestly think that a time out would be a good fix. If it takes unusually long it should time-out and should then try again.

@rafaelbn

This comment has been minimized.

Show comment
Hide comment
@rafaelbn

rafaelbn commented Nov 20, 2015

@rafaelbn

This comment has been minimized.

Show comment
Hide comment
@rafaelbn

rafaelbn Nov 20, 2015

@yelizariev it's much more effective if you make a PR to Odoo fixing it.
Also you can make a PR to https://github.com/OCA/OCB fixing the problem and have it solve it
👍

rafaelbn commented Nov 20, 2015

@yelizariev it's much more effective if you make a PR to Odoo fixing it.
Also you can make a PR to https://github.com/OCA/OCB fixing the problem and have it solve it
👍

@pedrobaeza

This comment has been minimized.

Show comment
Hide comment
@pedrobaeza
Contributor

pedrobaeza commented Nov 20, 2015

@elicoidal

This comment has been minimized.

Show comment
Hide comment
@elicoidal

elicoidal Nov 20, 2015

@pedrobaeza This is actually not enough.

We have created a patch this week that actually works and we will submit it next week to Odoo.
This current fix actually fixed the TIMEOUT after the connection is established while you need to fix it for establishing the connection (you need to change the method IMAP4_SSL and add a timeout parameter).

Our experience in China: with current fix we had continuous times out. Since we have this fix (3 or 4 days), we have no issue anymore.
We have to package a fix and submit to Odoo next week or if refused prepare a module for the OCA

elicoidal commented Nov 20, 2015

@pedrobaeza This is actually not enough.

We have created a patch this week that actually works and we will submit it next week to Odoo.
This current fix actually fixed the TIMEOUT after the connection is established while you need to fix it for establishing the connection (you need to change the method IMAP4_SSL and add a timeout parameter).

Our experience in China: with current fix we had continuous times out. Since we have this fix (3 or 4 days), we have no issue anymore.
We have to package a fix and submit to Odoo next week or if refused prepare a module for the OCA

@pedrobaeza

This comment has been minimized.

Show comment
Hide comment
@pedrobaeza

pedrobaeza Nov 20, 2015

Contributor

OK, let me know

Contributor

pedrobaeza commented Nov 20, 2015

OK, let me know

@elicoidal

This comment has been minimized.

Show comment
Hide comment
@elicoidal

elicoidal commented Nov 20, 2015

@daramousk

This comment has been minimized.

Show comment
Hide comment
@daramousk

daramousk Apr 24, 2017

Contributor

@elicoidal Has a final fix been provided on this ?

Contributor

daramousk commented Apr 24, 2017

@elicoidal Has a final fix been provided on this ?

@elicoidal

This comment has been minimized.

Show comment
Hide comment
@elicoidal

elicoidal Apr 24, 2017

@daramousk We made a PR which was not accepted: #9884

elicoidal commented Apr 24, 2017

@daramousk We made a PR which was not accepted: #9884

@pedrobaeza

This comment has been minimized.

Show comment
Hide comment
@pedrobaeza

pedrobaeza Apr 4, 2018

Contributor

@elicoidal maybe you can prepare a similar patch for v10/v11 or master. Any way, I see you are covering only IMAP, but not POP3.

Contributor

pedrobaeza commented Apr 4, 2018

@elicoidal maybe you can prepare a similar patch for v10/v11 or master. Any way, I see you are covering only IMAP, but not POP3.

@elicoidal

This comment has been minimized.

Show comment
Hide comment
@elicoidal

elicoidal Apr 5, 2018

@seb-elico could you have a look to propose a patch.
@pedrobaeza patch on v8 was not accepted: do you have any idea why? If we redo similar patch we might end up in the same situation

elicoidal commented Apr 5, 2018

@seb-elico could you have a look to propose a patch.
@pedrobaeza patch on v8 was not accepted: do you have any idea why? If we redo similar patch we might end up in the same situation

@Yenthe666

This comment has been minimized.

Show comment
Hide comment
@Yenthe666

Yenthe666 Apr 5, 2018

Contributor

@elicoidal the PR was denied & automatically closed because at that point Odoo 8 was no longer official supported.
In short: nobody really looked at your PR (automated message) and somebody should have a look when you make it against 9/10/11. Sorry for that closure 😞

Contributor

Yenthe666 commented Apr 5, 2018

@elicoidal the PR was denied & automatically closed because at that point Odoo 8 was no longer official supported.
In short: nobody really looked at your PR (automated message) and somebody should have a look when you make it against 9/10/11. Sorry for that closure 😞

@elicoidal

This comment has been minimized.

Show comment
Hide comment
@elicoidal

elicoidal Apr 5, 2018

@Yenthe666 Well noted. We will have a look for v10 for the moment which is our running version

elicoidal commented Apr 5, 2018

@Yenthe666 Well noted. We will have a look for v10 for the moment which is our running version

@Yenthe666

This comment has been minimized.

Show comment
Hide comment
@Yenthe666

Yenthe666 Apr 5, 2018

Contributor

Perfect. Let me know when you've made a PR. 😉

Contributor

Yenthe666 commented Apr 5, 2018

Perfect. Let me know when you've made a PR. 😉

@seb-elico

This comment has been minimized.

Show comment
Hide comment
@seb-elico

seb-elico Apr 8, 2018

Contributor

@seb-elico could you have a look to propose a patch.
@pedrobaeza patch on v8 was not accepted: do you have any idea why? If we redo similar patch we might end up in the same situation

@elicoidal the PR was denied & automatically closed because at that point Odoo 8 was no longer official supported.
In short: nobody really looked at your PR (automated message) and somebody should have a look when you make it against 9/10/11. Sorry for that closure disappointed

@elicoidal @Yenthe666 I confirm the patch #9884 has been closed after 2 years since v8 wasn't maintained anymore. I'll try to provide a new patch for master ASAP. Hopefully, it could be merged back to v11/v10/v9 as the code is pretty much the same.

Contributor

seb-elico commented Apr 8, 2018

@seb-elico could you have a look to propose a patch.
@pedrobaeza patch on v8 was not accepted: do you have any idea why? If we redo similar patch we might end up in the same situation

@elicoidal the PR was denied & automatically closed because at that point Odoo 8 was no longer official supported.
In short: nobody really looked at your PR (automated message) and somebody should have a look when you make it against 9/10/11. Sorry for that closure disappointed

@elicoidal @Yenthe666 I confirm the patch #9884 has been closed after 2 years since v8 wasn't maintained anymore. I'll try to provide a new patch for master ASAP. Hopefully, it could be merged back to v11/v10/v9 as the code is pretty much the same.

@Yenthe666

This comment has been minimized.

Show comment
Hide comment
@Yenthe666

Yenthe666 Apr 8, 2018

Contributor

@seb-elico don't forget the guidelines. If its a big you can create a PR for the lowest supported version where you have it in. If it is a new feature or addition in Odoo it should be only against master.

Contributor

Yenthe666 commented Apr 8, 2018

@seb-elico don't forget the guidelines. If its a big you can create a PR for the lowest supported version where you have it in. If it is a new feature or addition in Odoo it should be only against master.

@yelizariev

This comment has been minimized.

Show comment
Hide comment
@yelizariev

yelizariev Apr 9, 2018

Contributor

If its a bag

Contributor

yelizariev commented Apr 9, 2018

If its a bag

@seb-elico

This comment has been minimized.

Show comment
Hide comment
@seb-elico

seb-elico Apr 9, 2018

Contributor

If its a bug

@Yenthe666 Thanks for the reminder. I'm not the one to judge if not having a timeout is a bug though. IMO, adding a timeout is a new feature. Even if, in this case, it's to avoid having the cron run indefinitely, which could be considered as a bug. It's a question of interpretation. I would appreciate to see this patch back-ported obviously!

Contributor

seb-elico commented Apr 9, 2018

If its a bug

@Yenthe666 Thanks for the reminder. I'm not the one to judge if not having a timeout is a bug though. IMO, adding a timeout is a new feature. Even if, in this case, it's to avoid having the cron run indefinitely, which could be considered as a bug. It's a question of interpretation. I would appreciate to see this patch back-ported obviously!

@rafaelbn

This comment has been minimized.

Show comment
Hide comment
@rafaelbn

rafaelbn Apr 13, 2018

I will tell my grandchildren about this 😄

There was a BUG and a contributor made the PR in 2015 #9884 but one (1) year after, author close it because the version was not supported!!! OMG!

Maybe if @yelizariev would have made the PR in July 2015 Odoo would have merged and not closed? 😆

rafaelbn commented Apr 13, 2018

I will tell my grandchildren about this 😄

There was a BUG and a contributor made the PR in 2015 #9884 but one (1) year after, author close it because the version was not supported!!! OMG!

Maybe if @yelizariev would have made the PR in July 2015 Odoo would have merged and not closed? 😆

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