r113408 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113407‎ | r113408 | r113409 >
Date:21:44, 8 March 2012
Author:saper
Status:resolved (Comments)
Tags:1.19 
Comment:
PostgreSQL: Improve SQL error handling

After a query error, PostgreSQL transaction is aborted
until it's terminated or the query is closed.

All further queries result in:

ERROR: current transaction is aborted, commands ignored
until end of transaction block

Those subsequent errors are ignored by double fault handling in
DatabaseBase::reportQueryError but they cause all localization
of error messages to fail (unable to issue queries to message
tables) and errors lke


This resulted in a broken MediaWiki screen with

<databaseerror>
<dberrortext>

instead of localized error message.

We need to fully reset database connection because after
pg_connection_reset() various session parameters need to
be set again (like "search_path"), otherwise tables will not be
found.

ERROR: relation "msg_resource" does not exist
ERROR: relation "l10n_cache" does not exist
Modified paths:
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabasePostgres.php
——@@ -159,7 +159,6 @@
160160 return;
161161 }
162162
163 - $this->close();
164163 $this->mServer = $server;
165164 $port = $wgDBport;
166165 $this->mUser = $user;
——@@ -177,10 +176,14 @@
178177 if ( $port != false && $port != '' ) {
179178 $connectVars['port'] = $port;
180179 }
181 - $connectString = $this->makeConnectionString( $connectVars, PGSQL_CONNECT_FORCE_NEW );
 180+ $this->connectString = $this->makeConnectionString( $connectVars, PGSQL_CONNECT_FORCE_NEW );
 181+ $this->reOpen();
 182+ }
182183
 184+ function reOpen() {
 185+ $this->close();
183186 $this->installErrorHandler();
184 - $this->mConn = pg_connect( $connectString );
 187+ $this->mConn = pg_connect( $this->connectString );
185188 $phpError = $this->restoreErrorHandler();
186189
187190 if ( !$this->mConn ) {
——@@ -253,6 +256,13 @@
254257 return $this->mLastResult;
255258 }
256259
 260+ function reportQueryError( $error, $errno, $sql, $fname, $tempIgnore = false ) {
 261+ $this->rollback( __METHOD__ );
 262+ $this->reOpen();
 263+ parent::reportQueryError( $error, $errno, $sql, $fname, $tempIgnore );
 264+ }
 265+
 266+
257267 function queryIgnore( $sql, $fname = 'DatabasePostgres::queryIgnore' ) {
258268 return $this->query( $sql, $fname, true );
259269 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r113487Handle PostgreSQL transaction errors and improve schema detection...saper17:24, 9 March 2012

Comments

#Comment by Saper (talk | contribs)   02:32, 9 March 2012

$this->reOpen(); seems to break emulation of "SELECT IGNORE":

For example, deleting a page you watch with leaving "Watch this page" checked fails

Without $this->reOpen(); the connection seems to loose "search_path" after ROLLBACK on error: 

Query trunk (17) (slave): SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:searchresults-title'  LIMIT 1
Some query >>SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:searchresults-title'  LIMIT 1  << returned [NULL]
Query trunk (18) (slave): SELECT /* LinkCache::addLinkObj Saper */  page_id,page_len,page_is_redirect,page_latest  FROM "page"  WHERE page_namespace = '0' AND page_title = ''''  LIMIT 1
Some query >>SELECT /* LinkCache::addLinkObj Saper */  page_id,page_len,page_is_redirect,page_latest  FROM "page"  WHERE page_namespace = '0' AND page_title = ''''  LIMIT 1  << returned [NULL]
parseQuery received: '
parseQuery returned: ''''
Query trunk (19) (slave): SELECT /*  Saper */ to_tsquery('''')
Some query >>SELECT /*  Saper */ to_tsquery('''')<< returned [false]
Query trunk (20) (slave): ROLLBACK
Some query >>ROLLBACK<< returned [NULL]
SQL ERROR: ERROR:  syntax error in tsquery: "'"
Query trunk (21) (slave): BEGIN
Some query >>BEGIN<< returned [NULL]
Query trunk (22) (slave): SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:databaseerror'  LIMIT 1
Some query >>SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:databaseerror'  LIMIT 1  << returned [false]
Query trunk (23) (slave): ROLLBACK
Some query >>ROLLBACK<< returned [NULL]
SQL ERROR (ignored): ERROR:  relation "l10n_cache" does not exist
LINE 1: ...ELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cach...
                                                             ^
Query trunk (24) (slave): BEGIN
Some query >>BEGIN<< returned [NULL]
Query trunk (25) (slave): SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:dberrortext'  LIMIT 1
Some query >>SELECT /* LCStore_DB::get Saper */  lc_value  FROM "l10n_cache"  WHERE lc_lang = 'pl' AND lc_key = 'messages:dberrortext'  LIMIT 1  << returned [false]
Query trunk (26) (slave): ROLLBACK
#Comment by Saper (talk | contribs)   17:44, 15 March 2012

All issues so far resolved, now moving on to fix rest of PostgresUpdater problems.

#Comment by Krinkle (talk | contribs)   05:02, 5 July 2012

Reminder: Old revs in svn tagged for merging to 1.19. Please remove tag when done or no longer relevant.

Status & tagging log