From: Christian Aistleitner <christian@quelltextlich.at>
Date: Thu, 19 Feb 2015 23:13:07 +0000 (+0100)
Subject: Allow to set stub read buffer size for TextPassDumper
X-Git-Tag: 1.31.0-rc.0~12208^2
X-Git-Url: http://git.cyclocoop.org/fichier?a=commitdiff_plain;h=7f687cc858ed664a0e614f93b461d874f1cd1ef2;p=lhc%2Fweb%2Fwiklou.git

Allow to set stub read buffer size for TextPassDumper

The default buffer size of 512KB seems to unconveniently high for
checkpoint tests. To be able to speed up tests in a follow-up commit,
we allow callers to set the buffer size to use for reads of the stub.

Bug: T70653
Change-Id: Ib63e89fac2abaac8feb542839d4d8e53c917ebe1
---

diff --git a/maintenance/backupTextPass.inc b/maintenance/backupTextPass.inc
index 85ebd51bfe..d83f1fccc1 100644
--- a/maintenance/backupTextPass.inc
+++ b/maintenance/backupTextPass.inc
@@ -48,6 +48,8 @@ class TextPassDumper extends BackupDumper {
 	protected $maxConsecutiveFailedTextRetrievals = 200;
 	protected $failureTimeout = 5; // Seconds to sleep after db failure
 
+	protected $bufferSize = 524288; // In bytes. Maximum size to read from the stub in on go.
+
 	protected $php = "php";
 	protected $spawn = false;
 
@@ -186,6 +188,10 @@ class TextPassDumper extends BackupDumper {
 		$url = $this->processFileOpt( $val, $param );
 
 		switch ( $opt ) {
+			case 'buffersize':
+				// Lower bound for xml reading buffer size is 4 KB
+				$this->bufferSize = max( intval( $val ), 4 * 1024 );
+				break;
 			case 'prefetch':
 				require_once "$IP/maintenance/backupPrefetch.inc";
 				$this->prefetch = new BaseDump( $url );
@@ -368,12 +374,11 @@ class TextPassDumper extends BackupDumper {
 		xml_set_character_data_handler( $parser, array( &$this, 'characterData' ) );
 
 		$offset = 0; // for context extraction on error reporting
-		$bufferSize = 512 * 1024;
 		do {
 			if ( $this->checkIfTimeExceeded() ) {
 				$this->setTimeExceeded();
 			}
-			$chunk = fread( $input, $bufferSize );
+			$chunk = fread( $input, $this->bufferSize );
 			if ( !xml_parse( $parser, $chunk, feof( $input ) ) ) {
 				wfDebug( "TextDumpPass::readDump encountered XML parsing error\n" );
 
diff --git a/maintenance/dumpTextPass.php b/maintenance/dumpTextPass.php
index 7c176071ad..bde5a07623 100644
--- a/maintenance/dumpTextPass.php
+++ b/maintenance/dumpTextPass.php
@@ -59,6 +59,8 @@ Options:
   --server=h  Force reading from MySQL server h
   --current	  Base ETA on number of pages in database instead of all revisions
   --spawn	  Spawn a subprocess for loading text records
+  --buffersize=<size> Buffer size in bytes to use for reading the stub.
+              (Default: 512KB, Minimum: 4KB)
   --help      Display this help message
 ENDS
 	);
diff --git a/tests/phpunit/maintenance/backupTextPassTest.php b/tests/phpunit/maintenance/backupTextPassTest.php
index 0a977dc29a..8ed8e1068f 100644
--- a/tests/phpunit/maintenance/backupTextPassTest.php
+++ b/tests/phpunit/maintenance/backupTextPassTest.php
@@ -3,13 +3,13 @@
 require_once __DIR__ . "/../../../maintenance/backupTextPass.inc";
 
 /**
- * Tests for page dumps of BackupDumper
+ * Tests for TextPassDumper that rely on the database
  *
  * @group Database
  * @group Dump
  * @covers TextPassDumper
  */
-class TextPassDumperTest extends DumpTestCase {
+class TextPassDumperDatabaseTest extends DumpTestCase {
 
 	// We'll add several pages, revision and texts. The following variables hold the
 	// corresponding ids.
@@ -615,3 +615,61 @@ class BackupTextPassTestModelHandler extends TextContentHandler {
 	}
 
 }
+
+/**
+ * Tests for TextPassDumper that do not rely on the database
+ *
+ * (As the Database group is only detected at class level (not method level), we
+ * cannot bring this test case's tests into the above main test case.)
+ *
+ * @group Dump
+ * @covers TextPassDumper
+ */
+class TextPassDumperDatabaselessTest extends MediaWikiLangTestCase {
+	/**
+	 * Ensures that setting the buffer size is effective.
+	 *
+	 * @dataProvider bufferSizeProvider
+	 */
+	function testBufferSizeSetting( $expected, $size, $msg ) {
+		$dumper = new TextPassDumperAccessor( array( "--buffersize=" . $size ) );
+		$this->assertEquals( $expected, $dumper->getBufferSize(), $msg);
+	}
+
+	/**
+	 * Ensures that setting the buffer size is effective.
+	 *
+	 * @dataProvider bufferSizeProvider
+	 */
+	function bufferSizeProvider() {
+		// expected, bufferSize to initialize with, message
+		return array(
+			array( 512 * 1024, 512 * 1024, "Setting 512KB is not effective" ),
+			array( 8192, 8192, "Setting 8KB is not effective" ),
+			array( 4096, 2048, "Could set buffer size below lower bound" )
+		);
+	}
+}
+
+/**
+ * Accessor for internal state of TextPassDumper
+ *
+ * Do not warrentless add getters here.
+ */
+class TextPassDumperAccessor extends TextPassDumper {
+	/**
+	 * Gets the bufferSize.
+	 *
+	 * If bufferSize setting does not work correctly, testCheckpoint... tests
+	 * fail and point in the wrong direction. To aid in troubleshooting when
+	 * testCheckpoint... tests break at some point in the future, we test the
+	 * bufferSize setting, hence need this accessor.
+	 *
+	 * (Yes, bufferSize is internal state of the TextPassDumper, but aiding
+	 * debugging of testCheckpoint... in the future seems to be worth testing
+	 * against it nonetheless.)
+	 */
+	public function getBufferSize() {
+		return $this->bufferSize;
+	}
+}