Apologies if this is already known, I couldn't spot anything in my skim of the issues.
If you have PostgreSQL and are using cursors, have partial profiling enabled in Excimer, and have the dbpersist option enabled then it's possible for Excimer to clear the cursors unintentionally mid-request due to the alternative database connection in Totara.
Whenever pgsql_native_moodle_database->connect() is called, it'll run CLOSE ALL to clear any hanging cursors if $CFG->dboptions['dbpersist'] = true; is enabled.
If dbpersist is enabled, creating a second connection to the database may not give you a second connection and you end up reusing the first, meaning you clear cursors on a connection that might be using them.
Contrived example, no Excimer-specific code but demo's the problem.
// Make my own cursor
$t = $DB->start_delegated_transaction();
$DB->execute('DECLARE curs1 CURSOR FOR SELECT id,username FROM {user} ORDER BY id', null);
// Run against the cursor - this works
$x = $DB->get_records_sql('FETCH 3 FROM curs1');
var_dump($x);
// Open a second DB connection
$cfg = $DB->export_dbconfig();
$DB2 = \moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary);
$DB2->connect($cfg->dbhost, $cfg->dbuser, $cfg->dbpass, $cfg->dbname, $cfg->prefix, $cfg->dboptions);
// If dboptions[dbpersist] is enabled, this will crash as the cursor was cleared
// If dboptions[dbpersist] is not enabled, this will be fine
$x = $DB->get_records_sql('FETCH 3 FROM curs1');
var_dump($x);
$t->allow_commit();
I ran into it because I was executing a long-running query with cursors while I had excimer running, and traced it down to the second connection resetting the cursors when excimer was saving a profile mid-request.
I believe when we create the second connection in get_altconnection() we should override any dbpersist option and disable it, I can't see any need for it to ever be true.
Actual reproduction script:
- Enable DB persist
- Enable partial profiling
- Run the following (just prints out usernames)
<?php
require_once __DIR__ . '/config.php';
global $DB;
require_login();
// Make sure sessions are closed so partial profiling will log
\core\session\manager::write_close();
// Let's create a long-running script
$t = $DB->start_delegated_transaction();
$DB->execute('DECLARE curs1 CURSOR FOR SELECT id,username FROM {user} ORDER BY id', null);
for ($i = 0; $i < 100; $i++) {
$x = $DB->get_record_sql('FETCH 1 FROM curs1');
echo $x->username ?? 'None';
echo '<br>';
sleep(1);
}
echo '<br>Done!</br>';
If I watch the currently profiling page (just refreshing it), as soon as the profile is written the test script will crash with the following error:
Debug info: ERROR: cursor "curs1" does not exist
FETCH 1 FROM curs1
[array (
)]
Error code: dmlreadexception
Stack trace:
line 552 of /lib/dml/moodle_database.php: dml_read_exception thrown
line 294 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
line 1004 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
line 1715 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql_raw()
line 1974 of /lib/dml/moodle_database.php: call to moodle_database->get_records_sql()
line 17 of /test.php: call to moodle_database->get_record_sql()
Apologies if this is already known, I couldn't spot anything in my skim of the issues.
If you have PostgreSQL and are using cursors, have partial profiling enabled in Excimer, and have the
dbpersistoption enabled then it's possible for Excimer to clear the cursors unintentionally mid-request due to the alternative database connection in Totara.Whenever
pgsql_native_moodle_database->connect()is called, it'll runCLOSE ALLto clear any hanging cursors if$CFG->dboptions['dbpersist'] = true;is enabled.If
dbpersistis enabled, creating a second connection to the database may not give you a second connection and you end up reusing the first, meaning you clear cursors on a connection that might be using them.Contrived example, no Excimer-specific code but demo's the problem.
I ran into it because I was executing a long-running query with cursors while I had excimer running, and traced it down to the second connection resetting the cursors when excimer was saving a profile mid-request.
I believe when we create the second connection in get_altconnection() we should override any dbpersist option and disable it, I can't see any need for it to ever be true.
Actual reproduction script:
If I watch the currently profiling page (just refreshing it), as soon as the profile is written the test script will crash with the following error: