Skip to content

Commit

Permalink
MDL-69451 dml: use same temptables for both rw and ro database handle
Browse files Browse the repository at this point in the history
moodle_read_slave_trait: when creating another handle, restore temptables
property that is clobbered by raw_connect().

Also a better condition for temptable related queries detection in
pgsql_native_moodle_database.

dml_pgsql_read_slave_test::test_temp_table(): use real db connection
if possible, otherwise skip the test.
  • Loading branch information
srdjan-catalyst committed Aug 6, 2021
1 parent 0d0e66d commit fa0eecd
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 11 deletions.
6 changes: 5 additions & 1 deletion lib/dml/moodle_read_slave_trait.php
Expand Up @@ -226,9 +226,13 @@ public function connect($dbhost, $dbuser, $dbpass, $dbname, $prefix, array $dbop
* @return void
*/
private function set_dbhwrite(): void {
// Late connect to read/write master if needed.
// Lazy connect to read/write master.
if (!$this->dbhwrite) {
$temptables = $this->temptables;
$this->raw_connect($this->pdbhost, $this->pdbuser, $this->pdbpass, $this->pdbname, $this->pprefix, $this->pdboptions);
if ($temptables) {
$this->temptables = $temptables; // Restore temptables, so we don't get separate sets for rw and ro.
}
$this->dbhwrite = $this->get_db_handle();
}
$this->set_db_handle($this->dbhwrite);
Expand Down
2 changes: 1 addition & 1 deletion lib/dml/pgsql_native_moodle_database.php
Expand Up @@ -295,7 +295,7 @@ protected function can_use_readonly(int $type, string $sql): bool {
}

// ... a nuisance - temptables use this.
if (preg_match('/\bpg_constraint/', $sql) && $this->temptables->get_temptables()) {
if (preg_match('/\bpg_catalog/', $sql) && $this->temptables->get_temptables()) {
return false;
}

Expand Down
51 changes: 42 additions & 9 deletions lib/dml/tests/dml_pgsql_read_slave_test.php
Expand Up @@ -117,25 +117,58 @@ public function test_read_pg_lock_table() : void {
* @return void
*/
public function test_temp_table() : void {
if (PHP_VERSION_ID >= 80000) {
// TODO MDL-71482 - there seems to be a bigger problem here than just failing test on PHP8.
$this->markTestSkipped();
global $DB;

if ($DB->get_dbfamily() != 'postgres') {
$this->markTestSkipped("Not postgres");
}

$DB = new read_slave_moodle_database_mock_pgsql();
// Open second connection.
$cfg = $DB->export_dbconfig();
if (!isset($cfg->dboptions)) {
$cfg->dboptions = [];
}
if (!isset($cfg->dboptions['readonly'])) {
$cfg->dboptions['readonly'] = [
'instance' => [$cfg->dbhost]
];
}

$this->assertEquals(0, $DB->perf_get_reads_slave());
// Get a separate disposable db connection handle with guaranteed 'readonly' config.
$db2 = moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary);
$db2->connect($cfg->dbhost, $cfg->dbuser, $cfg->dbpass, $cfg->dbname, $cfg->prefix, $cfg->dboptions);

$dbman = $db2->get_manager();

$dbman = $DB->get_manager();
$table = new xmldb_table('silly_test_table');
$table->add_field('id', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, XMLDB_SEQUENCE);
$table->add_field('msg', XMLDB_TYPE_CHAR, 255);
$table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']);
$dbman->create_temp_table($table);

$DB->get_columns('silly_test_table');
$DB->get_records('silly_test_table');
$this->assertEquals(0, $DB->perf_get_reads_slave());
// We need to go through the creation proces twice.
// create_temp_table() performs some reads before the temp table is created.
// First time around those reads should go to ro ...
$reads = $db2->perf_get_reads_slave();

$db2->get_columns('silly_test_table');
$db2->get_records('silly_test_table');
$this->assertEquals($reads, $db2->perf_get_reads_slave());

$table2 = new xmldb_table('silly_test_table2');
$table2->add_field('id', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, XMLDB_SEQUENCE);
$table2->add_field('msg', XMLDB_TYPE_CHAR, 255);
$table2->add_key('primary', XMLDB_KEY_PRIMARY, ['id']);
$dbman->create_temp_table($table2);

// ... but once the first temp table is created no more ro reads should occur.
$db2->get_columns('silly_test_table2');
$db2->get_records('silly_test_table2');
$this->assertEquals($reads, $db2->perf_get_reads_slave());

// Make database driver happy.
$dbman->drop_table($table2);
$dbman->drop_table($table);
}

/**
Expand Down

0 comments on commit fa0eecd

Please sign in to comment.