From e0c8403a31231842223e431a82bef690e73dfe09 Mon Sep 17 00:00:00 2001 From: "Krzysztof Magusiak (krma)" Date: Fri, 31 Jan 2025 16:42:01 +0000 Subject: [PATCH] [IMP] base: ir.cron documentation New function and description of how to write cron jobs and related best-practices. task-4472661 closes odoo/documentation#12077 Related: odoo/odoo#197781 Signed-off-by: Antoine Vandevenne (anv) --- .../development/coding_guidelines.rst | 87 ++++++++++++++---- .../developer/reference/backend/actions.rst | 88 +++++++++++++++---- .../reference/backend/orm/changelog.rst | 6 ++ 3 files changed, 146 insertions(+), 35 deletions(-) diff --git a/content/contributing/development/coding_guidelines.rst b/content/contributing/development/coding_guidelines.rst index 07a7c17abd..253092955e 100644 --- a/content/contributing/development/coding_guidelines.rst +++ b/content/contributing/development/coding_guidelines.rst @@ -658,9 +658,15 @@ Never commit the transaction ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The Odoo framework is in charge of providing the transactional context for -all RPC calls. The principle is that a new database cursor is opened at the -beginning of each RPC call, and committed when the call has returned, just -before transmitting the answer to the RPC client, approximately like this: +all RPC calls. +All ``cr.commit()`` calls outside of the server framework must +have an **explicit comment** explaining why they are absolutely necessary, why +they are indeed correct, and why they do not break the transactions. Otherwise +they can and will be removed! + +The principle is that a new database cursor is opened at the beginning of each +RPC call, and committed when the call has returned, just before transmitting the +answer to the RPC client, approximately like this: .. code-block:: python @@ -671,7 +677,7 @@ before transmitting the answer to the RPC client, approximately like this: try: res = pool.execute_cr(cr, uid, obj, method, *args, **kw) cr.commit() # all good, we commit - except Exception: + except Exception: # try to be more specific cr.rollback() # error, rollback everything atomically raise finally: @@ -682,8 +688,7 @@ If any error occurs during the execution of the RPC call, the transaction is rolled back atomically, preserving the state of the system. Similarly, the system also provides a dedicated transaction during the execution -of tests suites, so it can be rolled back or not depending on the server -startup options. +of tests suites and scheduled actions. The consequence is that if you manually call ``cr.commit()`` anywhere there is a very high chance that you will break the system in various ways, because you @@ -697,9 +702,9 @@ among others: during the transaction) Here is the very simple rule: - You should **NEVER** call ``cr.commit()`` yourself, **UNLESS** you have - created your own database cursor explicitly! And the situations where you - need to do that are exceptional! + You should **NEVER** call ``cr.commit()`` or ``cr.rollback()`` yourself, + **UNLESS** you have explicitly created your own database cursor! + And the situations in which you need to do this are exceptional! And by the way if you did create your own cursor, then you need to handle error cases and proper rollback, as well as properly close the cursor when @@ -707,20 +712,66 @@ Here is the very simple rule: And contrary to popular belief, you do not even need to call ``cr.commit()`` in the following situations: + - in the ``_auto_init()`` method of an *models.Model* object: this is taken -care of by the addons initialization method, or by the ORM transaction when -creating custom models + care of by the addons initialization method, or by the ORM transaction when + creating custom models - in reports: the ``commit()`` is handled by the framework too, so you can -update the database even from within a report + update the database even from within a report - within *models.Transient* methods: these methods are called exactly like -regular *models.Model* ones, within a transaction and with the corresponding -``cr.commit()/rollback()`` at the end + regular *models.Model* ones, within a transaction and with the corresponding + ``cr.commit()/rollback()`` at the end - etc. (see general rule above if you are in doubt!) -All ``cr.commit()`` calls outside of the server framework from now on must -have an **explicit comment** explaining why they are absolutely necessary, why -they are indeed correct, and why they do not break the transactions. Otherwise -they can and will be removed ! +Avoid catching exceptions +~~~~~~~~~~~~~~~~~~~~~~~~~ + +Catch only specific exceptions, and avoid overly broad exception handling. +Uncaught exceptions will be logged and handled properly by the framework. + +You should be specific about the types you catch and handle them +accordingly, and you should limit the scope of your try-catch block as much +as possible. + +.. code-block:: python + + # BAD CODE + try: + do_something() + except Exception as e: + # if we caught a ValidationError, we did not rollback and we left the + # ORM in an undefined state + _logger.warning(e) + +For scheduled actions, you should rollback the changes if you catch errors and +wish to continue. Scheduled actions run in a separate transaction, so you can +rollback or commit directly when you signal progress. + +.. seealso:: + :ref:`reference/actions/cron` + +If you must handle framework exceptions, you must use **savepoints** +to isolate your function as much as possible. +This will flush the computations when entering the block and rollback changes +properly in case of exceptions. + +.. code-block:: python + + try: + with self.env.cr.savepoint(): + do_stuff() + except ...: + ... + +.. warning:: + + After you start more than 64 savepoints during a single transaction, + PostgreSQL will slow down. + In all cases, if the server runs replicas, savepoints have a huge overhead. + If you process records and savepoint in a loop, for example when processing + records one by one for a batch, limit the size of the batch. + If you have more records, the function should maybe become a scheduled job + or you have to accept the performance penalty. Use translation method correctly ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/content/developer/reference/backend/actions.rst b/content/developer/reference/backend/actions.rst index 1f3858ef05..e0d508ff29 100644 --- a/content/developer/reference/backend/actions.rst +++ b/content/developer/reference/backend/actions.rst @@ -413,6 +413,8 @@ how the POS interface works. Scheduled Actions (``ir.cron``) =============================== +.. automodule:: odoo.addons.base.models.ir_cron + Actions triggered automatically on a predefined frequency. ``name`` @@ -442,34 +444,84 @@ Actions triggered automatically on a predefined frequency. Priority of the action when executing multiple actions at the same time -Advanced use: Batching +Writing cron functions ---------------------- -When executing a scheduled action, it's recommended to try batching progress in order -to avoid hogging a worker for a long period of time and possibly running into timeout exceptions. +When running a scheduled action, it's recommended that you try to batch the +progress in order to avoid blocking a worker for a long period of time and +possibly run into timeout exceptions. Therefore, you should split the processing +so that each call makes progress on some of the work to be done. -Odoo provides a simple API for scheduled action batching; +When writing such a function, you should focus on processing a single batch. +A batch should process one or many records and should generally take no more +than *a few seconds*. + +Work is committed by the framework after each batch. The framework will +call the function as many times as necessary to process the remaining work. +Do not reschedule yourself the job. + +.. automethod:: IrCron._commit_progress .. code-block:: python - self.env['ir.cron']._notify_progress(done=XX:int, remaining=XX:int) + def _cron_do_something(self, *, limit=300): # limit: allows for tweaking + domain = [('state', '=', 'ready')] + records = self.search(domain, limit=limit) + records.do_something() + # notify progression + remaining = 0 if len(records) == limit else self.search_count(domain) + self.env['ir.cron']._commit_progress(len(records), remaining=remaining) -This method allows the scheduler to know if progress was made and whether there is -still remaining work that must be done. +In some cases, you may want to share resources between multiple batches or +manage the loop yourself to handle exceptions. +In this case, you should inform the scheduler of the progress of your work +by calling :func:`IrCron._commit_progress` and checking the result. The progress +function returns the number of seconds remaining for the call; if it is 0, you +must return as soon as possible. -By default, if the API is used, the scheduler tries to process 10 batches in one sitting. -If there are still remaining tasks after those 10 batches, a new cron call will be executed as -soon as possible. - -Advanced use: Triggers ----------------------- - -For more complex use cases, Odoo provides a more advanced way to trigger -scheduled actions directly from business code. +The following is an example of how to commit after each record that is +processed, while keeping the connection open. .. code-block:: python - action_record._trigger(at=XX:date) + def _cron_do_something(self): + assert self.env.context.get('cron_id'), "Run only inside cron jobs" + domain = [('state', '=', 'ready')] + records = self.search(domain) + self.env['ir.cron']._commit_progress(remaining=len(records)) + + with open_some_connection() as conn: + for record in records: + # You may have other needs; we do some common stuff here: + # - lock record (also checks existence) + # - prefetch: break prefetch in this case, we process one record + # - filtered_domain: record may have changed + record = record.try_lock_for_update().filtered_domain(domain) + if not record: + continue + # Processing the batch here... + try + record.do_something(conn) + if not self.env['ir.cron']._commit_progress(1): + break + except Exception: + # if you handle exceptions, the default stategy is to + # rollback first the error + self.env.cr.rollback() + _logger.warning(...) + # you may commit some status using _commit_progress + +Running cron functions +---------------------- + +You should not call cron functions directly. +There are two ways to run functions: + +.. automethod:: IrCron.method_direct_trigger +.. automethod:: IrCron._trigger + +Testing of a cron function should be done by calling +:func:`IrCron.method_direct_trigger` in the registry test mode. Security -------- @@ -481,3 +533,5 @@ correct functioning of your scheduled actions. it will skip its current execution and be considered as failed. - If a scheduled action fails its execution five consecutive times over a period of at least seven days, it will be deactivated and will notify the DB admin. +- A hard-limit exists for the cron execution at the database level after which + the process executing cron jobs is killed. diff --git a/content/developer/reference/backend/orm/changelog.rst b/content/developer/reference/backend/orm/changelog.rst index 6e26fd97d1..ac6e123384 100644 --- a/content/developer/reference/backend/orm/changelog.rst +++ b/content/developer/reference/backend/orm/changelog.rst @@ -4,6 +4,12 @@ Changelog ========= +Odoo Online version 18.3 +======================== + +- New cron API for notifying progress with batch commits. + See `#197781 `_. + Odoo Online version 18.2 ========================