Merge "externalstore: cleanup ExternalStoreDB::getTable() and fix callers"

3 files changed, 32 insertions, 26 deletions

diff --git a/includes/externalstore/ExternalStoreDB.php b/includes/externalstore/ExternalStoreDB.php
index 44160c650cb1..b9527adec762 100644
--- a/includes/externalstore/ExternalStoreDB.php
+++ b/includes/externalstore/ExternalStoreDB.php
@@ -115,11 +115,14 @@ class ExternalStoreDB extends ExternalStoreMedium {
 	 * @inheritDoc
 	 */
 	public function store( $location, $data ) {
+		$blobsTable = $this->getTable( $location );
+
 		$dbw = $this->getPrimary( $location );
 		$dbw->newInsertQueryBuilder()
-			->insertInto( $this->getTable( $dbw, $location ) )
+			->insertInto( $blobsTable )
 			->row( [ 'blob_text' => $data ] )
 			->caller( __METHOD__ )->execute();
+
 		$id = $dbw->insertId();
 		if ( !$id ) {
 			throw new ExternalStoreException( __METHOD__ . ': no insert ID' );
@@ -213,28 +216,25 @@ class ExternalStoreDB extends ExternalStoreMedium {
 	}
 
 	/**
-	 * Get the 'blobs' table name for this database
+	 * Get the configured blobs table name for this database
+	 *
+	 * Typically, a suffix like "_clusterX" can be used to facilitate clean merging of
+	 * read-only storage clusters by simply cloning tables to the new cluster servers.
 	 *
-	 * @param \Wikimedia\Rdbms\IReadableDatabase $db
-	 * @param string|null $cluster Cluster name
-	 * @return string Table name ('blobs' by default)
+	 * @param string $cluster Cluster name
+	 * @return string Unqualified table name (e.g. "blobs_cluster32" or default "blobs")
+	 * @internal Only for use within ExternalStoreDB and its core maintenance scripts
 	 */
-	public function getTable( $db, $cluster = null ) {
-		if ( $cluster !== null ) {
-			$lb = $this->getLoadBalancer( $cluster );
-			$info = $lb->getServerInfo( ServerInfo::WRITER_INDEX );
-			if ( isset( $info['blobs table'] ) ) {
-				return $info['blobs table'];
-			}
-		}
+	public function getTable( string $cluster ) {
+		$lb = $this->getLoadBalancer( $cluster );
+		$info = $lb->getServerInfo( ServerInfo::WRITER_INDEX );
 
-		return 'blobs';
+		return $info['blobs table'] ?? 'blobs';
 	}
 
 	/**
 	 * Create the appropriate blobs table on this cluster
 	 *
-	 * @see getTable()
 	 * @since 1.34
 	 * @param string $cluster
 	 */
@@ -254,9 +254,8 @@ class ExternalStoreDB extends ExternalStoreMedium {
 			throw new RuntimeException( "Failed to read '$sqlFilePath'." );
 		}
 
-		$rawTable = $this->getTable( $dbw, $cluster ); // e.g. "blobs_cluster23"
-		$encTable = $dbw->tableName( $rawTable );
-
+		$blobsTable = $this->getTable( $cluster );
+		$encTable = $dbw->tableName( $blobsTable );
 		$sqlWithReplacedVars = str_replace(
 			[ '/*$wgDBprefix*/blobs', '/*_*/blobs' ],
 			[ $encTable, $encTable ],
@@ -268,7 +267,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
 				$sqlWithReplacedVars,
 				$dbw::QUERY_CHANGE_SCHEMA,
 				'CREATE',
-				$rawTable,
+				$blobsTable,
 				$sqlWithReplacedVars
 			),
 			__METHOD__
@@ -304,12 +303,15 @@ class ExternalStoreDB extends ExternalStoreMedium {
 
 		$this->logger->debug( __METHOD__ . ": cache miss on $cacheID" );
 
+		$blobsTable = $this->getTable( $cluster );
+
 		$dbr = $this->getReplica( $cluster );
 		$ret = $dbr->newSelectQueryBuilder()
 			->select( 'blob_text' )
-			->from( $this->getTable( $dbr, $cluster ) )
+			->from( $blobsTable )
 			->where( [ 'blob_id' => $id ] )
 			->caller( __METHOD__ )->fetchField();
+
 		if ( $ret === false ) {
 			// Try the primary DB
 			$this->logger->warning( __METHOD__ . ": primary DB fallback on $cacheID" );
@@ -318,7 +320,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
 			$dbw = $this->getPrimary( $cluster );
 			$ret = $dbw->newSelectQueryBuilder()
 				->select( 'blob_text' )
-				->from( $this->getTable( $dbw, $cluster ) )
+				->from( $blobsTable )
 				->where( [ 'blob_id' => $id ] )
 				->caller( __METHOD__ )->fetchField();
 			ScopedCallback::consume( $scope );
@@ -345,10 +347,12 @@ class ExternalStoreDB extends ExternalStoreMedium {
 	 *   Unlocated ids are not represented
 	 */
 	private function batchFetchBlobs( $cluster, array $ids ) {
+		$blobsTable = $this->getTable( $cluster );
+
 		$dbr = $this->getReplica( $cluster );
 		$res = $dbr->newSelectQueryBuilder()
 			->select( [ 'blob_id', 'blob_text' ] )
-			->from( $this->getTable( $dbr, $cluster ) )
+			->from( $blobsTable )
 			->where( [ 'blob_id' => array_keys( $ids ) ] )
 			->caller( __METHOD__ )
 			->fetchResultSet();
@@ -366,7 +370,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
 			$dbw = $this->getPrimary( $cluster );
 			$res = $dbw->newSelectQueryBuilder()
 				->select( [ 'blob_id', 'blob_text' ] )
-				->from( $this->getTable( $dbr, $cluster ) )
+				->from( $blobsTable )
 				->where( [ 'blob_id' => array_keys( $ids ) ] )
 				->caller( __METHOD__ )
 				->fetchResultSet();
diff --git a/maintenance/storage/checkStorage.php b/maintenance/storage/checkStorage.php
index 5d1a85e9b366..75c6c722db7f 100644
--- a/maintenance/storage/checkStorage.php
+++ b/maintenance/storage/checkStorage.php
@@ -236,7 +236,7 @@ class CheckStorage extends Maintenance {
 				foreach ( $externalConcatBlobs as $cluster => $xBlobIds ) {
 					$blobIds = array_keys( $xBlobIds );
 					$extDb = $this->dbStore->getReplica( $cluster );
-					$blobsTable = $this->dbStore->getTable( $extDb );
+					$blobsTable = $this->dbStore->getTable( $cluster );
 					$res = $extDb->newSelectQueryBuilder()
 						->select( [ 'blob_id' ] )
 						->from( $blobsTable )
@@ -435,7 +435,7 @@ class CheckStorage extends Maintenance {
 		foreach ( $externalConcatBlobs as $cluster => $oldIds ) {
 			$blobIds = array_keys( $oldIds );
 			$extDb = $this->dbStore->getReplica( $cluster );
-			$blobsTable = $this->dbStore->getTable( $extDb );
+			$blobsTable = $this->dbStore->getTable( $cluster );
 			$headerLength = strlen( self::CONCAT_HEADER );
 			$res = $extDb->newSelectQueryBuilder()
 				->select( [ 'blob_id', "LEFT(blob_text, $headerLength) AS header" ] )
diff --git a/maintenance/storage/trackBlobs.php b/maintenance/storage/trackBlobs.php
index 0e3ba0dd54af..8fe041b36d24 100644
--- a/maintenance/storage/trackBlobs.php
+++ b/maintenance/storage/trackBlobs.php
@@ -331,6 +331,8 @@ class TrackBlobs extends Maintenance {
 
 		$dbw = $this->getPrimaryDB();
 		$lbFactory = $this->getServiceContainer()->getDBLoadBalancerFactory();
+		$dbStore = $this->getServiceContainer()->getExternalStoreFactory()->getStore( 'DB' );
+		'@phan-var ExternalStoreDB $dbStore'; /** @var ExternalStoreDB $dbStore */
 
 		foreach ( $this->clusters as $cluster ) {
 			echo "Searching for orphan blobs in $cluster...\n";
@@ -345,7 +347,7 @@ class TrackBlobs extends Maintenance {
 				}
 				continue;
 			}
-			$table = $extDB->getLBInfo( 'blobs table' ) ?? 'blobs';
+			$table = $dbStore->getTable( $cluster );
 			if ( !$extDB->tableExists( $table, __METHOD__ ) ) {
 				echo "No blobs table on cluster $cluster\n";
 				continue;