From da009ed5fb3caace984434071ce20c4105c92886 Mon Sep 17 00:00:00 2001
From: Ariel Glenn <ariel@users.mediawiki.org>
Date: Wed, 16 Jun 2010 20:12:29 +0000
Subject: [PATCH] distinguish failed text retrieval from empty text,
 consolidate text retrieval retry code, die after
 maxConsecutiveFailedTextRetrievals (of separate revisions)

---
 maintenance/dumpTextPass.php | 86 ++++++++++++++++++++++--------------
 maintenance/fetchText.php    | 21 ++++++++-
 2 files changed, 72 insertions(+), 35 deletions(-)

diff --git a/maintenance/dumpTextPass.php b/maintenance/dumpTextPass.php
index f83ab1a13d..75fd9ab59e 100644
--- a/maintenance/dumpTextPass.php
+++ b/maintenance/dumpTextPass.php
@@ -38,7 +38,9 @@ class TextPassDumper extends BackupDumper {
 	var $prefetchCount = 0;
 	
 	var $failures = 0;
-	var $maxFailures = 200;
+	var $maxFailures = 5;
+    var $failedTextRetrievals = 0;
+	var $maxConsecutiveFailedTextRetrievals = 200;
 	var $failureTimeout = 5; // Seconds to sleep after db failure
 
 	var $php = "php";
@@ -203,11 +205,50 @@ class TextPassDumper extends BackupDumper {
 	}
 	
 	private function doGetText( $id ) {
-		if ( $this->spawn ) {
-			return $this->getTextSpawned( $id );
-		} else {
-			return $this->getTextDbSafe( $id );
+
+		$this->failures = 0;
+		$ex = new MWException( "Graceful storage failure" );
+		while (true) {
+			if ( $this->spawn ) {
+				if ($this->failures) {
+					// we don't know why it failed, could be the child process 
+					// borked, could be db entry busted, could be db server out to lunch, 
+					// so cover all bases
+					$this->closeSpawn();
+					$this->openSpawn();
+				}
+				$text =  $this->getTextSpawned( $id );
+			} else {
+				$text =  $this->getTextDbSafe( $id );
+			}
+			if ( $text === false ) {
+				$this->failures++;
+				if ( $this->failures > $this->maxFailures) {
+					$this->progress( "Failed to retrieve revision text for text id ".
+									 "$id after $this->maxFailures tries, giving up" );
+					// were there so many bad retrievals in a row we want to bail?
+				    // at some point we have to declare the dump irretrievably broken
+					$this->failedTextRetrievals++;
+					if ($this->failedTextRetrievals > $this->maxConsecutiveFailedTextRetrievals) {
+						throw $ex;
+					}
+					else {
+						// would be nice to return something better to the caller someday,
+						// log what we know about the failure and about the revision
+						return("");
+					}
+				} else {
+					$this->progress( "Error $this->failures " .
+								 "of allowed $this->maxFailures retrieving revision text for text id $id! " .
+								 "Pausing $this->failureTimeout seconds before retry..." );
+					sleep( $this->failureTimeout );
+				}
+			} else {
+				$this->failedTextRetrievals= 0;
+				return( $text );
+			}
 		}
+
 	}
 	
 	/**
@@ -223,19 +264,7 @@ class TextPassDumper extends BackupDumper {
 			} catch ( DBQueryError $ex ) {
 				$text = false;
 			}
-			if ( $text === false ) {
-				$this->failures++;
-				if ( $this->failures > $this->maxFailures ) {
-					throw $ex;
-				} else {
-					$this->progress( "Database failure $this->failures " .
-						"of allowed $this->maxFailures for revision $id! " .
-						"Pausing $this->failureTimeout seconds..." );
-					sleep( $this->failureTimeout );
-				}
-			} else {
-				return $text;
-			}
+			return $text;
 		}
 	}
 	
@@ -264,21 +293,9 @@ class TextPassDumper extends BackupDumper {
 			// First time?
 			$this->openSpawn();
 		}
-		while ( true ) {
-			
-			$text = $this->getTextSpawnedOnce( $id );
-			if ( !is_string( $text ) ) {
-				$this->progress( "Database subprocess failed. Respawning..." );
-				
-				$this->closeSpawn();
-				sleep( $this->failureTimeout );
-				$this->openSpawn();
-				
-				continue;
-			}
-			wfRestoreWarnings();
-			return $text;
-		}
+		$text = $this->getTextSpawnedOnce( $id );
+		wfRestoreWarnings();
+		return $text;
 	}
 	
 	function openSpawn() {
@@ -354,6 +371,9 @@ class TextPassDumper extends BackupDumper {
 		if ( $len === false ) return false;
 		
 		$nbytes = intval( $len );
+		// actual error, not zero-length text
+		if ($nbytes < 0 ) return false;
+
 		$text = "";
 		
 		// Subprocess may not send everything at once, we have to loop.
diff --git a/maintenance/fetchText.php b/maintenance/fetchText.php
index 55b64d2f10..b4db7243f8 100644
--- a/maintenance/fetchText.php
+++ b/maintenance/fetchText.php
@@ -28,7 +28,17 @@ class FetchText extends Maintenance {
 		$this->mDescription = "Fetch the revision text from an old_id";
 	}
 
-	public function execute() {
+	/*
+     * returns a string containing the following in order:
+     *   textid
+	 *   \n
+	 *   length of text (-1 on error = failure to retrieve/unserialize/gunzip/etc)
+	 *   \n
+	 *   text  (may be empty)
+	 *
+	 * note that that the text string itself is *not* followed by newline
+	 */
+	 public function execute() {
 		$db = wfGetDB( DB_SLAVE );
 		$stdin = $this->getStdin();
 		while ( !feof( $stdin ) ) {
@@ -39,7 +49,14 @@ class FetchText extends Maintenance {
 			}
 			$textId = intval( $line );
 			$text = $this->doGetText( $db, $textId );
-			$this->output( $textId . "\n" . strlen( $text ) . "\n" . $text );
+			if ($text === false) {
+				# actual error, not zero-length text
+				$textLen = "-1";
+			}
+			else {
+				$textLen = strlen($text);
+			}
+			$this->output( $textId . "\n" . $textLen . "\n" . $text );
 		}
 	}
 	
-- 
2.20.1