[8.0] job runner #52
[8.0] job runner #52
Conversation
|
|
||
| def set_job_enqueued(self, uuid): | ||
| with closing(self.conn.cursor()) as cr: | ||
| cr.execute("UPDATE queue_job SET state=%s, date_enqueued=NOW() " |
There was a problem hiding this comment.
the date_enqueued must be truncated to 'seconds'
date_enqueued=date_trunc('seconds', now()::timestamp)otherwise Odoo will not be able to read it and will raise ValueError
File "/home/lmi/projects/openerp/openerp-deldrive-buildout/addons-connector/connector/queue/model.py", line 113, in _change_job_state
job = storage.load(job.uuid)
File "/home/lmi/projects/openerp/openerp-deldrive-buildout/addons-connector/connector/queue/job.py", line 265, in load
stored.date_enqueued, DEFAULT_SERVER_DATETIME_FORMAT)
File "/usr/lib/python2.7/_strptime.py", line 328, in _strptime
data_string[found.end():])
ValueError: unconverted data remains: .024856
81e55ad to
4f493b6
Compare
| _logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class PriorityQueue: |
There was a problem hiding this comment.
Can you change the classes to new-style classes?
|
As said before, this is a brilliant work. I added some suggestions or interrogations. |
Here is a PR: acsone#1 Tests will be executed when |
|
BTW could you rebase your branch? |
|
Started the implementation of the channels configuration: acsone#2 |
a900aec to
d1f25e9
Compare
|
I rebased. Thanks for the review @guewen. |
| channel_name, uuid) | ||
| channel = self._root_channel | ||
| job = ChannelJob(db_name, channel, uuid, | ||
| seq, date_created, priority, eta) |
There was a problem hiding this comment.
Be careful. In Odoo python date, datetime and time are returned as string by psycopg2. (see https://github.com/odoo/odoo/blob/8.0/openerp/sql_db.py#L61 and http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.new_type). If you assign a string as eta to your ChannelJob you'll get a Comparison error between str and datetime when popping you job at line #L418
while not self.capacity or len(self._running) < self.capacity:
job = self._queue.pop(now=datetime.now())due to #L275
def pop(self, now):
if len(self._eta_queue) and self._eta_queue[0].eta <= now:There was a problem hiding this comment.
@lmignon thanks. I fixed that.
|
👍 it's really a great improvement! |
|
@sbidoul @lmignon We tested the runner (the 7.0 backport) on a production server with ~15000 jobs per day. We encountered "too many connections" issues with postgresql. I think that's only a matter of misconfiguration on our side. Did you issued similar issues ? Do you know how many connections may the runners use to the fullest? |
|
@guewen We also encountered the same issue with (+100k jobs/d). If I remember well, you have to change the db_maxconn parameter in your configuration (by default 64). |
This NOTIFY is not necessary and makes the purge of the job queue very slow. Thanks to @acsonelmi for the suggestion.
Odoo indeed adapts the psycopg2 type mappings to use strings for datetime.
…able The presence of ODOO_CONNECTOR_CHANNELS is enough to make the runner start in place of the workers.
Since the runner is now a thread in the Odoo main process, such a situation should occur only with a pathologically wrong configuration, so no need to worry about that... unless proven otherwise ;)
So the channel module does not make assumptions on the type of the eta field, so we use the odoo-ish way to get now in a database-compatible format, so it is more easily testable.
7d64a30 to
24f54e0
Compare
|
I rebased to get the latest travis config. There are 3 todo's left, which IMO can be left for future work:
So from my point of view it's ready to merge if and when Travis manages to gives back a green status. |
Absolutely deserved. |
| supports job channels. You should try the job runner first | ||
| and fall back to using workers in case the runner does not | ||
| work (sic) for you, in which case we will very much appreciate | ||
| a github issue describing the problems you encoutered. |
|
I spotted 2 very little things in the doc (inline comments), apart that, all good! |
|
I'm really impressed by this PR and the educational aspects of the doctects associated to each new concept. The connector module is one of my favorite OCA addons and a killer feature for Odoo/OCA. IMO it's ready to be merged. 👍 |
I can't express it better. Many thanks for this contribution. |
|
Cool. Thanks guys! |
|
@sbidoul is this runner available on 7.0 branch? |
This is an alternative to connector workers that
For more information on how to use it, see the docstring in runner.py.
For more information on job channels, see the docstring of the Channel class in channels.py.
It is safe to try in real deployments because it is fully compatible and it is easy to switch between the new runner and workers.
TODO: