MediaWiki r42992 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r42991‎ | r42992 (on ViewVC)‎ | r42993 >
Date:12:06, 1 November 2008
Author:tstarling
Status:old (Comments)
Tags:
Comment:
* Added $wgDebugLogPrefix, to allow users (or in this case, multiprocess command-line scripts) to set a debug log line prefix
* Improved blob_tracking.sql docs
* Allow testCompression.php to run until happy instead of a specified revision count
* Added docs for some debugging globals in DefaultSettings.php

In experimental script recompressTracked.php:
* Fixed crippling bugs, seems to work now. Needs more testing.
* Improved usage documentation
* Use DiffHistoryBlob by default if available
* Set a process-specific log prefix, added more debugging output
* Optimised commit() slightly by merging the locking read queries using IN()
* Fixed finishIncompleteMoves() so doPage() acts on only that page. Also run finishIncompleteMoves() for orphan lists.
Modified paths:

Diff [purge]

Index: trunk/phase3/maintenance/storage/recompressTracked.php
——@@ -4,9 +4,13 @@
55 require( dirname( __FILE__ ) .'/../commandLine.inc' );
66
77 if ( count( $args ) < 1 ) {
8 - echo "Usage: php recompressTracked.php <cluster> [... <cluster>...]\n";
9 - echo "Moves blobs indexed by trackBlobs.php to a specified list of destination
10 -clusters, and recompresses them in the process. Restartable.\n";
 8+ echo "Usage: php recompressTracked.php [options] <cluster> [... <cluster>...]
 9+Moves blobs indexed by trackBlobs.php to a specified list of destination clusters, and recompresses them in the process. Restartable.
 10+
 11+Options:
 12+ --procs <procs> Set the number of child processes (default 8)
 13+ --copy-only Copy only, do not update the text table. Restart without this option to complete.
 14+";
1115 exit( 1 );
1216 }
1317
——@@ -18,17 +22,16 @@
1923 var $batchSize = 1000;
2024 var $reportingInterval = 10;
2125 var $numProcs = 8;
 26+ var $useDiff, $pageBlobClass, $orphanBlobClass;
2227 var $slavePipes, $slaveProcs, $prevSlaveId;
23 - var $blobClass = 'DiffHistoryBlob';
2428 var $copyOnly = false;
2529 var $isChild = false;
2630 var $slaveId = false;
2731 var $store;
2832
29 - static $optionsWithArgs = array( 'procs', 'class' );
 33+ static $optionsWithArgs = array( 'procs', 'slave-id' );
3034 static $cmdLineOptionMap = array(
3135 'procs' => 'numProcs',
32 - 'class' => 'blobClass',
3336 'copy-only' => 'copyOnly',
3437 'child' => 'isChild',
3538 'slave-id' => 'slaveId',
——@@ -53,14 +56,18 @@
5457 $this->$name = $value;
5558 }
5659 $this->store = new ExternalStoreDB;
 60+ if ( !$this->isChild ) {
 61+ $GLOBALS['wgDebugLogPrefix'] = "RCT M: ";
 62+ } elseif ( $this->slaveId !== false ) {
 63+ $GLOBALS['wgDebugLogPrefix'] = "RCT {$this->slaveId}: ";
 64+ }
 65+ $this->useDiff = function_exists( 'xdiff_string_bdiff' );
 66+ $this->pageBlobClass = $this->useDiff ? 'DiffHistoryBlob' : 'ConcatenatedGzipHistoryBlob';
 67+ $this->orphanBlobClass = 'ConcatenatedGzipHistoryBlob';
5768 }
5869
5970 function debug( $msg ) {
60 - if ( $this->slaveId !== false ) {
61 - $msg = "{$this->slaveId}: $msg";
62 - }
63 - $msg .= "\n";
64 - wfDebug( $msg );
 71+ wfDebug( "$msg\n" );
6572 }
6673
6774 /**
——@@ -146,7 +153,7 @@
147154 array( 'file', '/dev/stderr', 'w' )
148155 );
149156 wfSuppressWarnings();
150 - $proc = proc_open( $cmd, $spec, $pipes );
 157+ $proc = proc_open( "$cmd --slave-id $i", $spec, $pipes );
151158 wfRestoreWarnings();
152159 if ( !$proc ) {
153160 echo "Error opening slave process\n";
——@@ -299,6 +306,7 @@
300307 * Main entry point for worker processes
301308 */
302309 function executeChild() {
 310+ $this->debug( 'starting' );
303311 $this->syncDBs();
304312
305313 while ( !feof( STDIN ) ) {
——@@ -306,6 +314,7 @@
307315 if ( $line == '' ) {
308316 continue;
309317 }
 318+ $this->debug( $line );
310319 $args = explode( ' ', $line );
311320 $cmd = array_shift( $args );
312321 switch ( $cmd ) {
——@@ -325,15 +334,21 @@
326335 * Move tracked text in a given page
327336 */
328337 function doPage( $pageId ) {
 338+ $title = Title::newFromId( $pageId );
 339+ if ( $title ) {
 340+ $titleText = $title->getPrefixedText();
 341+ } else {
 342+ $titleText = '[deleted]';
 343+ }
329344 $dbr = wfGetDB( DB_SLAVE );
330345
331346 // Finish any incomplete transactions
332347 if ( !$this->copyOnly ) {
333 - $this->finishIncompleteMoves();
 348+ $this->finishIncompleteMoves( array( 'bt_page' => $pageId ) );
334349 }
335350
336351 $startId = 0;
337 - $trx = new CgzCopyTransaction( $this );
 352+ $trx = new CgzCopyTransaction( $this, $this->pageBlobClass );
338353
339354 while ( true ) {
340355 $res = $dbr->select(
——@@ -343,7 +358,7 @@
344359 'bt_page' => $pageId,
345360 'bt_text_id > ' . $dbr->addQuotes( $startId ),
346361 'bt_moved' => 0,
347 - 'bt_new_url' => '',
 362+ 'bt_new_url IS NULL',
348363 'bt_text_id=old_id',
349364 ),
350365 __METHOD__,
——@@ -372,12 +387,15 @@
373388
374389 // Queue it
375390 if ( !$trx->addItem( $text, $row->bt_text_id ) ) {
 391+ $this->debug( "$titleText: committing blob with " . $trx->getSize() . " items" );
376392 $trx->commit();
377 - $trx = new CgzCopyTransaction( $this );
 393+ $trx = new CgzCopyTransaction( $this, $this->pageBlobClass );
378394 }
379395 }
380396 $startId = $row->bt_text_id;
381397 }
 398+
 399+ $this->debug( "$titleText: committing blob with " . $trx->getSize() . " items" );
382400 $trx->commit();
383401 }
384402
——@@ -420,18 +438,18 @@
421439 * This function completes any moves that only have done bt_new_url. This
422440 * can happen when the script is interrupted, or when --copy-only is used.
423441 */
424 - function finishIncompleteMoves() {
 442+ function finishIncompleteMoves( $conds ) {
425443 $dbr = wfGetDB( DB_SLAVE );
426444
427445 $startId = 0;
 446+ $conds = array_merge( $conds, array(
 447+ 'bt_moved' => 0,
 448+ 'bt_new_url IS NOT NULL'
 449+ ));
428450 while ( true ) {
429451 $res = $dbr->select( 'blob_tracking',
430452 '*',
431 - array(
432 - 'bt_text_id > ' . $dbr->addQuotes( $startId ),
433 - 'bt_moved' => 0,
434 - "bt_new_url <> ''",
435 - ),
 453+ array_merge( $conds, array( 'bt_text_id > ' . $dbr->addQuotes( $startId ) ) ),
436454 __METHOD__,
437455 array(
438456 'ORDER BY' => 'bt_text_id',
——@@ -441,6 +459,7 @@
442460 if ( !$res->numRows() ) {
443461 break;
444462 }
 463+ $this->debug( 'Incomplete: ' . $row->numRows() . ' rows' );
445464 foreach ( $res as $row ) {
446465 $this->moveTextRow( $row->bt_text_id, $row->bt_new_url );
447466 }
——@@ -471,7 +490,10 @@
472491 * Move an orphan text_id to the new cluster
473492 */
474493 function doOrphanList( $textIds ) {
475 - $trx = new CgzCopyTransaction( $this );
 494+ // Finish incomplete moves
 495+ $this->finishIncompleteMoves( array( 'bt_text_id' => $textIds ) );
 496+
 497+ $trx = new CgzCopyTransaction( $this, $this->orphanBlobClass );
476498 foreach ( $textIds as $textId ) {
477499 $row = wfGetDB( DB_SLAVE )->selectRow( 'text', array( 'old_text', 'old_flags' ),
478500 array( 'old_id' => $textId ), __METHOD__ );
——@@ -482,10 +504,13 @@
483505 }
484506
485507 if ( !$trx->addItem( $text, $textId ) ) {
 508+ $this->debug( "[orphan]: committing blob with " . $trx->getSize() . " rows" );
486509 $trx->commit();
487 - $trx = new CgzCopyTransaction( $this );
 510+ $trx = new CgzCopyTransaction( $this, $this->orphanBlobClass );
488511 }
489512 }
 513+ $this->debug( "[orphan]: committing blob with " . $trx->getSize() . " rows" );
 514+ $trx->commit();
490515 }
491516 }
492517
——@@ -493,6 +518,7 @@
494519 * Class to represent a recompression operation for a single CGZ blob
495520 */
496521 class CgzCopyTransaction {
 522+ var $parent;
497523 var $blobClass;
498524 var $cgz;
499525 var $referrers;
——@@ -500,10 +526,11 @@
501527 /**
502528 * Create a transaction from a RecompressTracked object
503529 */
504 - function __construct( $parent ) {
505 - $this->blobClass = $parent->blobClass;
 530+ function __construct( $parent, $blobClass ) {
 531+ $this->blobClass = $blobClass;
506532 $this->cgz = false;
507533 $this->texts = array();
 534+ $this->parent = $parent;
508535 }
509536
510537 /**
——@@ -521,6 +548,10 @@
522549 return $this->cgz->isHappy();
523550 }
524551
 552+ function getSize() {
 553+ return count( $this->texts );
 554+ }
 555+
525556 /**
526557 * Recompress text after some aberrant modification
527558 */
——@@ -554,16 +585,16 @@
555586 // We do a locking read to prevent closer-run race conditions.
556587 $dbw = wfGetDB( DB_MASTER );
557588 $dbw->begin();
 589+ $res = $dbw->select( 'blob_tracking',
 590+ array( 'bt_text_id', 'bt_moved' ),
 591+ array( 'bt_text_id' => array_keys( $this->referrers ) ),
 592+ __METHOD__, array( 'FOR UPDATE' ) );
558593 $dirty = false;
559 - foreach ( $this->referrers as $textId => $hash ) {
560 - $moved = $dbw->selectField( 'blob_tracking', 'bt_moved',
561 - array( 'bt_text_id' => $textId ),
562 - __METHOD__,
563 - array( 'FOR UPDATE' )
564 - );
565 - if ( !$moved ) {
 594+ foreach ( $res as $row ) {
 595+ if ( $row->bt_moved ) {
566596 # This row has already been moved, remove it
567 - unset( $this->texts[$textId] );
 597+ $this->parent->debug( "TRX: conflict detected in old_id={$row->bt_text_id}" );
 598+ unset( $this->texts[$row->bt_text_id] );
568599 $dirty = true;
569600 }
570601 }
——@@ -574,7 +605,7 @@
575606 // All have been moved already
576607 if ( $originalCount > 1 ) {
577608 // This is suspcious, make noise
578 - echo "Warning: concurrent operation detected, are there two conflicting\n" .
 609+ echo "Warning: concurrent operation detected, are there two conflicting " .
579610 "processes running, doing the same job?\n";
580611 }
581612 return;
——@@ -616,9 +647,5 @@
617648 }
618649 }
619650 }
620 -
621 - function signalHandler() {
622 - $this->signalled = true;
623 - }
624651 }
625652
Index: trunk/phase3/maintenance/storage/testCompression.php
——@@ -15,7 +15,13 @@
1616 } else {
1717 $start = '19700101000000';
1818 }
19 -$limit = isset( $options['limit'] ) ? $options['limit'] : 10;
 19+if ( isset( $options['limit'] ) ) {
 20+ $limit = $options['limit'];
 21+ $untilHappy = false;
 22+} else {
 23+ $limit = 1000;
 24+ $untilHappy = true;
 25+}
2026 $type = isset( $options['type'] ) ? $options['type'] : 'ConcatenatedGzipHistoryBlob';
2127
2228
——@@ -43,16 +49,21 @@
4450 $uncompressedSize += strlen( $text );
4551 $hashes[$row->rev_id] = md5( $text );
4652 $keys[$row->rev_id] = $blob->addItem( $text );
 53+ if ( $untilHappy && !$blob->isHappy() ) {
 54+ break;
 55+ }
4756 }
4857
4958 $serialized = serialize( $blob );
5059 $t += microtime( true );
 60+#print_r( $blob->mDiffMap );
5161
52 -printf( "Compression ratio for %d revisions: %5.2f, %s -> %s\n",
53 - $res->numRows(),
 62+printf( "%s\nCompression ratio for %d revisions: %5.2f, %s -> %d\n",
 63+ $type,
 64+ count( $hashes ),
5465 $uncompressedSize / strlen( $serialized ),
5566 $wgLang->formatSize( $uncompressedSize ),
56 - $wgLang->formatSize( strlen( $serialized ) )
 67+ strlen( $serialized )
5768 );
5869 printf( "Compression time: %5.2f ms\n", $t * 1000 );
5970
Index: trunk/phase3/maintenance/storage/blob_tracking.sql
——@@ -4,10 +4,14 @@
55 CREATE TABLE /*$wgDBprefix*/blob_tracking (
66 -- page.page_id
77 -- This may be zero for orphan or deleted text
 8+ -- Note that this is for compression grouping only -- it doesn't need to be
 9+ -- accurate at the time recompressTracked is run. Operations such as a
 10+ -- delete/undelete cycle may make it inaccurate.
811 bt_page integer not null,
912
1013 -- revision.rev_id
1114 -- This may be zero for orphan or deleted text
 15+ -- Like bt_page, it does not need to be accurate when recompressTracked is run.
1216 bt_rev_id integer not null,
1317
1418 -- text.old_id
Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -195,6 +195,7 @@
196196 */
197197 function wfDebug( $text, $logonly = false ) {
198198 global $wgOut, $wgDebugLogFile, $wgDebugComments, $wgProfileOnly, $wgDebugRawPage;
 199+ global $wgDebugLogPrefix;
199200 static $recursion = 0;
200201
201202 static $cache = array(); // Cache of unoutputted messages
——@@ -227,6 +228,7 @@
228229 # Strip unprintables; they can switch terminal modes when binary data
229230 # gets dumped, which is pretty annoying.
230231 $text = preg_replace( '![\x00-\x08\x0b\x0c\x0e-\x1f]!', ' ', $text );
 232+ $text = $wgDebugLogPrefix . $text;
231233 wfErrorLog( $text, $wgDebugLogFile );
232234 }
233235 }
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -838,7 +838,6 @@
839839
840840 /**
841841 * Translation using MediaWiki: namespace.
842 - * This will increase load times by 25-60% unless memcached is installed.
843842 * Interface messages will be loaded from the database.
844843 */
845844 $wgUseDatabaseMessages = true;
——@@ -952,6 +951,16 @@
953952 $wgExtraSubtitle = '';
954953 $wgSiteSupportPage = ''; # A page where you users can receive donations
955954
 955+/**
 956+ * Set this to a string to put the wiki into read-only mode. The text will be
 957+ * used as an explanation to users.
 958+ *
 959+ * This prevents most write operations via the web interface. Cache updates may
 960+ * still be possible. To prevent database writes completely, use the read_only
 961+ * option in MySQL.
 962+ */
 963+$wgReadOnly = null;
 964+
956965 /***
957966 * If this lock file exists, the wiki will be forced into read-only mode.
958967 * Its contents will be shown to users as part of the read-only warning
——@@ -960,15 +969,42 @@
961970 $wgReadOnlyFile = false; ///< defaults to "{$wgUploadDirectory}/lock_yBgMBwiR";
962971
963972 /**
 973+ * Filename for debug logging.
964974 * The debug log file should be not be publicly accessible if it is used, as it
965 - * may contain private data. */
 975+ * may contain private data.
 976+ */
966977 $wgDebugLogFile = '';
967978
 979+/**
 980+ * Prefix for debug log lines
 981+ */
 982+$wgDebugLogPrefix = '';
 983+
 984+/**
 985+ * If true, instead of redirecting, show a page with a link to the redirect
 986+ * destination. This allows for the inspection of PHP error messages, and easy
 987+ * resubmission of form data. For developer use only.
 988+ */
968989 $wgDebugRedirects = false;
969 -$wgDebugRawPage = false; # Avoid overlapping debug entries by leaving out CSS
970990
 991+/**
 992+ * If true, log debugging data from action=raw.
 993+ * This is normally false to avoid overlapping debug entries due to gen=css and
 994+ * gen=js requests.
 995+ */
 996+$wgDebugRawPage = false;
 997+
 998+/**
 999+ * Send debug data to an HTML comment in the output.
 1000+ *
 1001+ * This may occasionally be useful when supporting a non-technical end-user. It's
 1002+ * more secure than exposing the debug log file to the web, since the output only
 1003+ * contains private data for the current user. But it's not ideal for development
 1004+ * use since data is lost on fatal errors and redirects.
 1005+ */
9711006 $wgDebugComments = false;
972 -$wgReadOnly = null;
 1007+
 1008+/** Does nothing. Obsolete? */
9731009 $wgLogQueries = false;
9741010
9751011 /**
——@@ -1027,7 +1063,8 @@
10281064 * same options.
10291065 *
10301066 * This can provide a significant speedup for medium to large pages,
1031 - * so you probably want to keep it on.
 1067+ * so you probably want to keep it on. Extensions that conflict with the
 1068+ * parser cache should disable the cache on a per-page basis instead.
10321069 */
10331070 $wgEnableParserCache = true;
10341071

Comments

#Comment by Aaron Schulz (talk | contribs)   00:58, 5 November 2008

'$row->numRows()' fixed in r42994

#Comment by Aaron Schulz (talk | contribs)   01:02, 5 November 2008

"array_merge( $conds, array( 'bt_text_id > ' . $dbr->addQuotes( $startId ) ) )"

Won't this keep adding more "bt_text_id > 'x'" clauses on to the WHERE?

#Comment by Tim Starling (talk | contribs)   09:34, 13 November 2008

No.

#Comment by Aaron Schulz (talk | contribs)   10:03, 13 November 2008

Nevermind. It's not setting $conds to that.

Status & tagging log

  • 15:36, 12 September 2011 Meno25 (talk | contribs) changed the status of r42992 [removed: resolved added: old]
Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/42992"