From 3753257c0ce81b76e9d0c3a52038c6711f6688dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 30 May 2023 14:21:31 +0200 Subject: [PATCH 1/2] docs(developer): Add new atomicRetry method to 27 developer documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../app_upgrade_guide/upgrade_to_27.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_27.rst b/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_27.rst index e58eb626c..426071559 100644 --- a/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_27.rst +++ b/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_27.rst @@ -60,6 +60,7 @@ Added APIs * ``isAppLoaded(string $app): bool`` Check if an app is loaded * ``loadApps(array $types = []): bool`` Loads all apps. If $types is set to non-empty array, only apps of those types will be loaded. * ``isType(string $app, array $types): bool`` Check if an app is of a specific type +* New method ``atomicRetry()`` as been added to the ``\OCP\AppFramework\Db\TTransactional`` trait as a wrapper around atomic() to retry database operations after a retryable exception like database deadlocks occurred (`nextcloud/server#38030 `_) Changed APIs ^^^^^^^^^^^^ From e7f9fa5ec608a01a34b4ca5484282233425587be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 9 Jun 2023 16:00:28 +0200 Subject: [PATCH 2/2] docs(developer): Add digging deeper section about deadlocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- developer_manual/digging_deeper/deadlock.rst | 103 +++++++++++++++++++ developer_manual/digging_deeper/index.rst | 1 + 2 files changed, 104 insertions(+) create mode 100644 developer_manual/digging_deeper/deadlock.rst diff --git a/developer_manual/digging_deeper/deadlock.rst b/developer_manual/digging_deeper/deadlock.rst new file mode 100644 index 000000000..696777b82 --- /dev/null +++ b/developer_manual/digging_deeper/deadlock.rst @@ -0,0 +1,103 @@ +========= +Deadlocks +========= + +Deadlocks are a classic problem in transactional databases, but **they +are not dangerous unless they are so frequent that you cannot run +certain transactions at all**. Normally, you must write your +applications so that they are always prepared to re-issue a transaction +if it gets rolled back because of a deadlock. + +Understanding the locking situation +----------------------------------- + +MySQL/MariaDB automatically detects transaction deadlocks and rolls back +a transaction or transactions to break the deadlock. It tries to pick +small transactions to roll back, where the size of a transaction is +determined by the number of rows inserted, updated, or deleted. + +In order to fix a deadlock you will need to get an understanding of the +scenarios where deadlocks occur in your application by analyzing the +Nextcloud log for patterns in the deadlock errors. The Nextcloud log +will only show the transaction that was rolled back in this case so in +order to properly understand the deadlock scenario it is required to +obtain further information from the database server. By default you can +find the last detected deadlock in the output of +``SHOW ENGINE INNODB STATUS`` however ``innodb_print_all_deadlocks`` can +be used as a setting to write all occurring deadlocks to the server +logs. Those give detailed insight about which queries are holding a lock +and are causing the query from Nextcloud to run into a deadlock. + +Mitigations +----------- + +There are basically 3 options on how deadlocks can be handled properly. +It is not critical for all use case to handle them, however as per +recommendation of MySQL/MariaDB the application should be prepared to +handle them properly. + +Ignoring deadlocks +^^^^^^^^^^^^^^^^^^ + +There are some scenarios where a deadlock could be safely ignored, like +when updating a timestamp that is likely already being updated by +another concurrent request. In this case developers can catch the +exception and ignore the failed transaction. + +- Potentially useful API wrapper: + https://github.com/nextcloud/server/pull/38030 +- Valid case example: https://github.com/nextcloud/server/pull/37820 + +:: + + try { + // Database transaction that runs into the deadlock + $qb->executeStatement(); + } catch (DbalException $e) { + // ignore the failure + $this->logger->info("Deadlock detected, but ignored", ['exception' => $e]); + } + +Retrying deadlocks +^^^^^^^^^^^^^^^^^^ + +In other cases it might be feasible to just retry the specific database +transactions. In this case the exception needs to be catched and the +transaction needs to be re-issued. It is recommended to limit the amount +if retries in case the deadlock occurring regularly. In this case you +may follow the next section. + +An example how to do that can be found in +https://github.com/nextcloud/server/pull/34302 + +Starting with Nextcloud 27 there is also a useful helper method +``atomicRetry`` which makes retrying transactions a lot simpler: + +:: + + class MyClass { + use \OCP\AppFramework\Db\TTransactional; + + public function myFunction() { + $this->atomicRetry(function() { + // Database transaction that runs into the deadlock + $qb->executeStatement(); + }, $this->connection, 5); + } + } + +Avoiding deadlocks +^^^^^^^^^^^^^^^^^^ + +While not always possible due to the concurrency that may happen on +Nextcloud, it might be feasible to refactor logic so that the load of +concurrent writes to a table or columns is reduced or only one request +may hold a lock at a table row at the same time. + +References +---------- + +* https://dev.mysql.com/doc/refman/8.0/en/innodb-deadlocks.html +* https://dev.mysql.com/doc/refman/8.0/en/innodb-deadlocks-handling.html +* https://percona.community/blog/2018/09/24/minimize-mysql-deadlocks-3-steps/ +* https://www.percona.com/blog/enable-innodb_print_all_deadlocks-parameter-to-get-all-deadlock-information-in-mysqld-error-log/ diff --git a/developer_manual/digging_deeper/index.rst b/developer_manual/digging_deeper/index.rst index 368ea6525..608dd25ba 100644 --- a/developer_manual/digging_deeper/index.rst +++ b/developer_manual/digging_deeper/index.rst @@ -38,3 +38,4 @@ Digging deeper profile user_migration profiler + deadlock