From 70d480ab7fac0380464ffb4e67bcbb7e43ea4950 Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Fri, 17 Jun 2022 11:00:43 +0100
Subject: [PATCH] Issue #2733675 by smccabe, murilohp, andregp, Johnny Santos,
ankithashetty, mglaman, jonathanshaw, daffie, alexpott, catch, froboy:
Warning when mysql is not set to READ-COMMITTED
---
.../scaffold/files/default.settings.php | 15 ++++
core/modules/mysql/mysql.install | 45 ++++++++++
.../tests/src/Functional/RequirementsTest.php | 88 +++++++++++++++++++
sites/default/default.settings.php | 15 ++++
4 files changed, 163 insertions(+)
create mode 100644 core/modules/mysql/mysql.install
create mode 100644 core/modules/mysql/tests/src/Functional/RequirementsTest.php
diff --git a/core/assets/scaffold/files/default.settings.php b/core/assets/scaffold/files/default.settings.php
index 72be7750b93a..53977afb9911 100644
--- a/core/assets/scaffold/files/default.settings.php
+++ b/core/assets/scaffold/files/default.settings.php
@@ -138,6 +138,21 @@
* request as needed. The fourth line creates a new database with a name of
* "extra".
*
+ * For MySQL, MariaDB or equivalent databases the 'isolation_level' option can
+ * be set. The recommended transaction isolation level for Drupal sites is
+ * 'READ COMMITTED'. The 'REPEATABLE READ' option is supported but can result
+ * in deadlocks, the other two options are 'READ UNCOMMITTED' and 'SERIALIZABLE'.
+ * They are available but not supported; use them at your own risk. For more
+ * info:
+ * https://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-isolation-levels.html
+ *
+ * On your settings.php, change the isolation level:
+ * @code
+ * $databases['default']['default']['init_commands'] = [
+ * 'isolation_level' => 'SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED',
+ * ];
+ * @endcode
+ *
* You can optionally set a prefix for all database table names by using the
* 'prefix' setting. If a prefix is specified, the table name will be prepended
* with its value. Be sure to use valid database characters only, usually
diff --git a/core/modules/mysql/mysql.install b/core/modules/mysql/mysql.install
new file mode 100644
index 000000000000..a585abc5517a
--- /dev/null
+++ b/core/modules/mysql/mysql.install
@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * @file
+ * Install, update and uninstall functions for the mysql module.
+ */
+
+use Drupal\Core\Database\Database;
+
+/**
+ * Implements hook_requirements().
+ */
+function mysql_requirements($phase) {
+ $requirements = [];
+
+ if ($phase === 'runtime') {
+ // Test with MySql databases.
+ if (Database::isActiveConnection()) {
+ $connection = Database::getConnection();
+
+ $query = 'SELECT @@SESSION.tx_isolation';
+ // The database variable "tx_isolation" has been removed in MySQL v8.0 and
+ // has been replaced by "transaction_isolation".
+ // @see https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_tx_isolation
+ if (!$connection->isMariaDb() && version_compare($connection->version(), '8.0.0-AnyName', '>')) {
+ $query = 'SELECT @@SESSION.transaction_isolation';
+ }
+
+ $isolation_level = $connection->query($query)->fetchField();
+
+ if ($isolation_level !== 'READ-COMMITTED') {
+ $requirements['mysql_transaction_level'] = [
+ 'title' => t('Database Isolation Level'),
+ 'severity' => REQUIREMENT_WARNING,
+ 'value' => t('Transaction Isolation Level: @value', ['@value' => $isolation_level]),
+ 'description' => t('For the best performance and to minimize locking issues, the READ-COMMITTED transaction isolation level is <a href=":performance_doc">recommended</a>.', [
+ ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level',
+ ]),
+ ];
+ }
+ }
+ }
+
+ return $requirements;
+}
diff --git a/core/modules/mysql/tests/src/Functional/RequirementsTest.php b/core/modules/mysql/tests/src/Functional/RequirementsTest.php
new file mode 100644
index 000000000000..e10c5f041566
--- /dev/null
+++ b/core/modules/mysql/tests/src/Functional/RequirementsTest.php
@@ -0,0 +1,88 @@
+<?php
+
+namespace Drupal\Tests\mysql\Functional;
+
+use Drupal\Core\Database\Database;
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests isolation level warning when the config is set in settings.php.
+ *
+ * @group mysql
+ */
+class RequirementsTest extends BrowserTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['mysql'];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected $defaultTheme = 'stark';
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ // The isolation_level option is only available for MySQL.
+ $connectionInfo = Database::getConnectionInfo();
+ if ($connectionInfo['default']['driver'] !== 'mysql') {
+ $this->markTestSkipped("This test does not support the {$connectionInfo['default']['driver']} database driver.");
+ }
+ }
+
+ /**
+ * Test the isolation level warning message on status page.
+ */
+ public function testIsolationLevelWarningNotDisplaying() {
+ $admin_user = $this->drupalCreateUser([
+ 'administer site configuration',
+ 'access site reports',
+ ]);
+ $this->drupalLogin($admin_user);
+
+ // Change the isolation level to force the warning message.
+ $this->writeIsolationLevelSettings('REPEATABLE READ');
+
+ // Check if the warning message is being displayed.
+ $this->drupalGet('admin/reports/status');
+ $elements = $this->xpath('//details[@class="system-status-report__entry"]//div[contains(text(), :text)]', [
+ ':text' => 'For the best performance and to minimize locking issues, the READ-COMMITTED',
+ ]);
+ $this->assertCount(1, $elements);
+ $this->assertStringStartsWith('Transaction Isolation Level', $elements[0]->getParent()->getText());
+
+ // Rollback the isolation level to read committed.
+ $this->writeIsolationLevelSettings('READ COMMITTED');
+
+ // Check if the warning message is gone.
+ $this->drupalGet('admin/reports/status');
+ $this->assertSession()->pageTextNotContains('Database Isolation Level');
+ $elements = $this->xpath('//details[@class="system-status-report__entry"]//div[contains(text(), :text)]', [
+ ':text' => 'For the best performance and to minimize locking issues, the READ-COMMITTED',
+ ]);
+
+ $this->assertCount(0, $elements);
+ }
+
+ /**
+ * Writes the isolation level in settings.php.
+ *
+ * @param string $isolation_level
+ * The isolation level.
+ */
+ private function writeIsolationLevelSettings(string $isolation_level) {
+ $settings['databases']['default']['default']['init_commands'] = (object) [
+ 'value' => [
+ 'isolation' => "SET SESSION TRANSACTION ISOLATION LEVEL {$isolation_level}",
+ ],
+ 'required' => TRUE,
+ ];
+ $this->writeSettings($settings);
+ }
+
+}
diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php
index 72be7750b93a..53977afb9911 100644
--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php
@@ -138,6 +138,21 @@
* request as needed. The fourth line creates a new database with a name of
* "extra".
*
+ * For MySQL, MariaDB or equivalent databases the 'isolation_level' option can
+ * be set. The recommended transaction isolation level for Drupal sites is
+ * 'READ COMMITTED'. The 'REPEATABLE READ' option is supported but can result
+ * in deadlocks, the other two options are 'READ UNCOMMITTED' and 'SERIALIZABLE'.
+ * They are available but not supported; use them at your own risk. For more
+ * info:
+ * https://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-isolation-levels.html
+ *
+ * On your settings.php, change the isolation level:
+ * @code
+ * $databases['default']['default']['init_commands'] = [
+ * 'isolation_level' => 'SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED',
+ * ];
+ * @endcode
+ *
* You can optionally set a prefix for all database table names by using the
* 'prefix' setting. If a prefix is specified, the table name will be prepended
* with its value. Be sure to use valid database characters only, usually
--
GitLab