From 0363b7826bffd29df144044f6a4334d365c00f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Fri, 25 Jan 2013 22:46:12 +0100 Subject: [PATCH 1/2] MDL-37734 use prefetching workaround for MARS transaction problems --- lib/dml/sqlsrv_native_moodle_database.php | 23 +++++++++- lib/dml/sqlsrv_native_moodle_recordset.php | 53 ++++++++++++++++++++-- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php index 30ceb2c8368..6fc935dc046 100644 --- a/lib/dml/sqlsrv_native_moodle_database.php +++ b/lib/dml/sqlsrv_native_moodle_database.php @@ -41,6 +41,8 @@ class sqlsrv_native_moodle_database extends moodle_database { protected $last_error_reporting; // To handle SQL*Server-Native driver default verbosity protected $temptables; // Control existing temptables (sqlsrv_moodle_temptables object) protected $collation; // current DB collation cache + /** @var array list of open recordsets */ + protected $recordsets = array(); /** * Constructor - instantiates the database, specifying if it's external (connect to other systems) or no (Moodle DB) @@ -789,7 +791,20 @@ class sqlsrv_native_moodle_database extends moodle_database { * @return sqlsrv_native_moodle_recordset */ protected function create_recordset($result) { - return new sqlsrv_native_moodle_recordset($result); + $rs = new sqlsrv_native_moodle_recordset($result, $this); + $this->recordsets[] = $rs; + return $rs; + } + + /** + * Do not use outside of recordset class. + * @internal + * @param sqlsrv_native_moodle_recordset $rs + */ + public function recordset_closed(sqlsrv_native_moodle_recordset $rs) { + if ($key = array_search($rs, $this->recordsets, true)) { + unset($this->recordsets[$key]); + } } /** @@ -1367,6 +1382,12 @@ class sqlsrv_native_moodle_database extends moodle_database { * @return void */ protected function begin_transaction() { + // Recordsets do not work well with transactions in SQL Server, + // let's prefetch the recordsets to memory to work around these problems. + foreach ($this->recordsets as $rs) { + $rs->transaction_starts(); + } + $this->query_start('native sqlsrv_begin_transaction', NULL, SQL_QUERY_AUX); $result = sqlsrv_begin_transaction($this->sqlsrv); $this->query_end($result); diff --git a/lib/dml/sqlsrv_native_moodle_recordset.php b/lib/dml/sqlsrv_native_moodle_recordset.php index e397e2e52a2..677c2d07ef7 100644 --- a/lib/dml/sqlsrv_native_moodle_recordset.php +++ b/lib/dml/sqlsrv_native_moodle_recordset.php @@ -31,9 +31,49 @@ class sqlsrv_native_moodle_recordset extends moodle_recordset { protected $rsrc; protected $current; - public function __construct($rsrc) { - $this->rsrc = $rsrc; + /** @var array recordset buffer */ + protected $buffer = null; + + /** @var sqlsrv_native_moodle_database */ + protected $db; + + public function __construct($rsrc, sqlsrv_native_moodle_database $db) { + $this->rsrc = $rsrc; $this->current = $this->fetch_next(); + $this->db = $db; + } + + /** + * Inform existing open recordsets that transaction + * is starting, this works around MARS problem described + * in MDL-37734. + */ + public function transaction_starts() { + if ($this->buffer !== null) { + $this->unregister(); + return; + } + if (!$this->rsrc) { + $this->unregister(); + return; + } + // This might eat memory pretty quickly... + raise_memory_limit('2G'); + $this->buffer = array(); + + while($next = $this->fetch_next()) { + $this->buffer[] = $next; + } + } + + /** + * Unregister recordset from the global list of open recordsets. + */ + private function unregister() { + if ($this->db) { + $this->db->recordset_closed($this); + $this->db = null; + } } public function __destruct() { @@ -47,6 +87,7 @@ class sqlsrv_native_moodle_recordset extends moodle_recordset { if (!$row = sqlsrv_fetch_array($this->rsrc, SQLSRV_FETCH_ASSOC)) { sqlsrv_free_stmt($this->rsrc); $this->rsrc = null; + $this->unregister(); return false; } @@ -69,7 +110,11 @@ class sqlsrv_native_moodle_recordset extends moodle_recordset { } public function next() { - $this->current = $this->fetch_next(); + if ($this->buffer === null) { + $this->current = $this->fetch_next(); + } else { + $this->current = array_shift($this->buffer); + } } public function valid() { @@ -82,5 +127,7 @@ class sqlsrv_native_moodle_recordset extends moodle_recordset { $this->rsrc = null; } $this->current = null; + $this->buffer = null; + $this->unregister(); } } From dd8a39a88170ca0c153e674ff8773eede6d364af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Tue, 29 Jan 2013 10:10:40 +0100 Subject: [PATCH 2/2] MDL-37734 add one more recordset isolation test --- lib/dml/tests/dml_test.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php index 5010b727934..1c93f9d1d3f 100644 --- a/lib/dml/tests/dml_test.php +++ b/lib/dml/tests/dml_test.php @@ -4444,6 +4444,35 @@ class dml_testcase extends database_driver_testcase { } $rs1->close(); $this->assertEquals(3, $i); + + // Test nested recordsets isolation without transaction. + $DB->delete_records($tablename); + $DB->insert_record($tablename, array('course'=>1)); + $DB->insert_record($tablename, array('course'=>2)); + $DB->insert_record($tablename, array('course'=>3)); + + $DB->delete_records($tablename2); + $DB->insert_record($tablename2, array('course'=>5)); + $DB->insert_record($tablename2, array('course'=>6)); + $DB->insert_record($tablename2, array('course'=>7)); + $DB->insert_record($tablename2, array('course'=>8)); + + $rs1 = $DB->get_recordset($tablename); + $i = 0; + foreach ($rs1 as $record1) { + $i++; + $rs2 = $DB->get_recordset($tablename2); + $j = 0; + foreach ($rs2 as $record2) { + $DB->set_field($tablename, 'course', $record1->course+1, array('id'=>$record1->id)); + $DB->set_field($tablename2, 'course', $record2->course+1, array('id'=>$record2->id)); + $j++; + } + $rs2->close(); + $this->assertEquals(4, $j); + } + $rs1->close(); + $this->assertEquals(3, $i); } function test_transactions_forbidden() {