mirror of
https://github.com/moodle/moodle.git
synced 2025-04-15 05:25:08 +02:00
MDL-73713 dml: moodle_read_slave_trait table written time adjustment
moodle_database::commit_delegated_transaction() calls core\event\manager::database_transaction_commited() which may invoke events that do database reads. This change adjusts tables last written time in moodle_read_slave_trait *before* calling moodle_database::commit_delegated_transaction().
This commit is contained in:
Srdjan
parent
34ce1463cc
commit
8a386d68f6
@ -326,6 +326,15 @@ trait moodle_read_slave_trait {
|
||||
if (isset($this->written[$tablename])) {
|
||||
if ($this->slavelatency) {
|
||||
$now = $now ?: microtime(true);
|
||||
// Paranoid check.
|
||||
if ($this->written[$tablename] === true) {
|
||||
debugging(
|
||||
"$tablename last written set to true outside transaction - should not happen!",
|
||||
DEBUG_DEVELOPER
|
||||
);
|
||||
$this->written[$tablename] = $now;
|
||||
return false;
|
||||
}
|
||||
if ($now - $this->written[$tablename] < $this->slavelatency) {
|
||||
return false;
|
||||
}
|
||||
@ -339,7 +348,6 @@ trait moodle_read_slave_trait {
|
||||
return true;
|
||||
case SQL_QUERY_INSERT:
|
||||
case SQL_QUERY_UPDATE:
|
||||
// If we are in transaction we cannot set the written time yet.
|
||||
$now = $this->slavelatency && !$this->transactions ? microtime(true) : true;
|
||||
foreach ($this->table_names($sql) as $tablename) {
|
||||
$this->written[$tablename] = $now;
|
||||
@ -364,23 +372,15 @@ trait moodle_read_slave_trait {
|
||||
* @throws dml_transaction_exception Creates and throws transaction related exceptions.
|
||||
*/
|
||||
public function commit_delegated_transaction(moodle_transaction $transaction) {
|
||||
parent::commit_delegated_transaction($transaction);
|
||||
|
||||
if ($this->transactions) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$this->slavelatency) {
|
||||
return;
|
||||
}
|
||||
|
||||
$now = null;
|
||||
foreach ($this->written as $tablename => $when) {
|
||||
if ($when === true) {
|
||||
$now = $now ?: microtime(true);
|
||||
if ($this->slavelatency && $this->written) {
|
||||
// Adjust the written time.
|
||||
$now = microtime(true);
|
||||
foreach ($this->written as $tablename => $when) {
|
||||
$this->written[$tablename] = $now;
|
||||
}
|
||||
}
|
||||
|
||||
parent::commit_delegated_transaction($transaction);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -21,12 +21,14 @@
|
||||
* @category dml
|
||||
* @copyright 2018 Srdjan Janković, Catalyst IT
|
||||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
|
||||
* @coversDefaultClass \moodle_temptables
|
||||
*/
|
||||
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
require_once(__DIR__.'/fixtures/read_slave_moodle_database_table_names.php');
|
||||
require_once(__DIR__.'/fixtures/read_slave_moodle_database_special.php');
|
||||
require_once(__DIR__.'/../../tests/fixtures/event_fixtures.php');
|
||||
|
||||
/**
|
||||
* DML read/read-write database handle use tests
|
||||
@ -36,7 +38,7 @@ require_once(__DIR__.'/fixtures/read_slave_moodle_database_special.php');
|
||||
* @copyright 2018 Catalyst IT
|
||||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
|
||||
*/
|
||||
class core_dml_read_slave_testcase extends base_testcase {
|
||||
class dml_read_slave_test extends \base_testcase {
|
||||
|
||||
/** @var float */
|
||||
static private $dbreadonlylatency = 0.8;
|
||||
@ -278,12 +280,15 @@ class core_dml_read_slave_testcase extends base_testcase {
|
||||
* so the latency parameter is applied properly.
|
||||
*
|
||||
* @return void
|
||||
* @covers ::can_use_readonly
|
||||
* @covers ::commit_delegated_transaction
|
||||
*/
|
||||
public function test_transaction() : void {
|
||||
public function test_transaction(): void {
|
||||
$DB = $this->new_db(true);
|
||||
|
||||
$this->assertNull($DB->get_dbhwrite());
|
||||
|
||||
$skip = false;
|
||||
$transaction = $DB->start_delegated_transaction();
|
||||
$now = microtime(true);
|
||||
$handle = $DB->get_records_sql("SELECT * FROM {table}");
|
||||
@ -304,11 +309,92 @@ class core_dml_read_slave_testcase extends base_testcase {
|
||||
|
||||
// Make sure enough time passes.
|
||||
sleep(1);
|
||||
} else {
|
||||
$skip = true;
|
||||
}
|
||||
|
||||
// Exceeded latency time, use ro handle.
|
||||
$handle = $DB->get_records_sql("SELECT * FROM {table}");
|
||||
$this->assert_readonly_handle($handle);
|
||||
|
||||
if ($skip) {
|
||||
$this->markTestSkipped("Delay too long to test write handle immediately after transaction");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Test readonly handle is not used with events
|
||||
* when the latency parameter is applied properly.
|
||||
*
|
||||
* @return void
|
||||
* @covers ::can_use_readonly
|
||||
* @covers ::commit_delegated_transaction
|
||||
*/
|
||||
public function test_transaction_with_events(): void {
|
||||
$this->with_global_db(function () {
|
||||
global $DB;
|
||||
|
||||
$DB = $this->new_db(true, ['test_ro'], read_slave_moodle_database_special::class);
|
||||
$DB->set_tables([
|
||||
'config_plugins' => [
|
||||
'columns' => [
|
||||
'plugin' => (object)['meta_type' => ''],
|
||||
]
|
||||
]
|
||||
]);
|
||||
|
||||
$this->assertNull($DB->get_dbhwrite());
|
||||
|
||||
$this->_called = false;
|
||||
$transaction = $DB->start_delegated_transaction();
|
||||
$now = microtime(true);
|
||||
|
||||
$observers = [
|
||||
[
|
||||
'eventname' => '\core_tests\event\unittest_executed',
|
||||
'callback' => function (\core_tests\event\unittest_executed $event) use ($DB, $now) {
|
||||
$this->_called = true;
|
||||
$this->assertFalse($DB->is_transaction_started());
|
||||
|
||||
// This condition should always evaluate true, however we need to
|
||||
// safeguard from an unaccounted delay that can break this test.
|
||||
if (microtime(true) - $now < 1 + self::$dbreadonlylatency) {
|
||||
// Not enough time passed, use rw handle.
|
||||
$handle = $DB->get_records_sql_p("SELECT * FROM {table}");
|
||||
$this->assertEquals('test_rw::test:test', $handle);
|
||||
|
||||
// Make sure enough time passes.
|
||||
sleep(1);
|
||||
} else {
|
||||
$this->markTestSkipped("Delay too long to test write handle immediately after transaction");
|
||||
}
|
||||
|
||||
// Exceeded latency time, use ro handle.
|
||||
$handle = $DB->get_records_sql_p("SELECT * FROM {table}");
|
||||
$this->assertEquals('test_ro::test:test', $handle);
|
||||
},
|
||||
'internal' => 0,
|
||||
],
|
||||
];
|
||||
\core\event\manager::phpunit_replace_observers($observers);
|
||||
|
||||
$handle = $DB->get_records_sql_p("SELECT * FROM {table}");
|
||||
// Use rw handle during transaction.
|
||||
$this->assertEquals('test_rw::test:test', $handle);
|
||||
|
||||
$handle = $DB->insert_record_raw('table', array('name' => 'blah'));
|
||||
// Introduce delay so we can check that table write timestamps
|
||||
// are adjusted properly.
|
||||
sleep(1);
|
||||
$event = \core_tests\event\unittest_executed::create([
|
||||
'context' => \context_system::instance(),
|
||||
'other' => ['sample' => 1]
|
||||
]);
|
||||
$event->trigger();
|
||||
$transaction->allow_commit();
|
||||
|
||||
$this->assertTrue($this->_called);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -26,7 +26,6 @@
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
require_once(__DIR__.'/read_slave_moodle_database.php');
|
||||
require_once(__DIR__.'/read_slave_moodle_recordset_special.php');
|
||||
|
||||
/**
|
||||
* Database driver mock test class that uses read_slave_moodle_recordset_special
|
||||
@ -50,6 +49,19 @@ class read_slave_moodle_database_special extends read_slave_moodle_database {
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns read_slave_moodle_database::get_records_sql()
|
||||
* For the tests where we need both fake result and dbhandle info.
|
||||
* @param string $sql the SQL select query to execute.
|
||||
* @param array $params array of sql parameters
|
||||
* @param int $limitfrom return a subset of records, starting at this point (optional).
|
||||
* @param int $limitnum return a subset comprising this many records (optional, required if $limitfrom is set).
|
||||
* @return string $handle handle property
|
||||
*/
|
||||
public function get_records_sql_p($sql, array $params = null, $limitfrom = 0, $limitnum = 0) {
|
||||
return parent::get_records_sql($sql, $params);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns fake recordset
|
||||
* @param string $sql
|
||||
@ -74,3 +86,46 @@ class read_slave_moodle_database_special extends read_slave_moodle_database {
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Database recordset mock test class
|
||||
*
|
||||
* @package core
|
||||
* @category dml
|
||||
* @copyright 2018 Catalyst IT
|
||||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
|
||||
*/
|
||||
class read_slave_moodle_recordset_special extends \moodle_recordset {
|
||||
/**
|
||||
* Iterator interface
|
||||
* @return void
|
||||
*/
|
||||
public function close() {
|
||||
}
|
||||
/**
|
||||
* Iterator interface
|
||||
* @return stdClass
|
||||
*/
|
||||
public function current() {
|
||||
return new stdClass();
|
||||
}
|
||||
/**
|
||||
* Iterator interface
|
||||
* @return void
|
||||
*/
|
||||
public function next() {
|
||||
}
|
||||
/**
|
||||
* Iterator interface
|
||||
* @return mixed
|
||||
*/
|
||||
public function key() {
|
||||
}
|
||||
/**
|
||||
* Iterator interface
|
||||
* @return bool
|
||||
*/
|
||||
public function valid() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user