Merge pull request #6910 from MrPetovan/task/6778-storage-move-loop

6778 Alternative 2: Wrap storage move in a loop

mod/admin.php

@@ -1200,7 +1200,9 @@ function admin_page_site_post(App $a)
 	 * @var $storagebackend \Friendica\Model\Storage\IStorage
 	 */
 	$storagebackend    = Strings::escapeTags(trim(defaults($_POST, 'storagebackend', '')));
-	StorageManager::setBackend($storagebackend);
+	if (!StorageManager::setBackend($storagebackend)) {
+		info(L10n::t('Invalid storage backend setting value.'));
+	}
 
 	// save storage backend form
 	if (!is_null($storagebackend) && $storagebackend != "") {

src/Core/Console/Storage.php

@@ -28,9 +28,10 @@ Synopsis
         Set current storage backend
             name        storage backend to use. see "list".
     
-    bin/console storage move [table]
+    bin/console storage move [table] [-n 5000]
         Move stored data to current storage backend.
             table       one of "photo" or "attach". default to both
+            -n          limit of processed entry batch size
 HELP;
 		return $help;
 	}
@@ -51,13 +52,13 @@ HELP;
 
 		switch ($this->args[0]) {
 			case 'list':
-			return $this->do_list();
+				return $this->doList();
 				break;
 			case 'set':
-			return $this->do_set();
+				return $this->doSet();
 				break;
 			case 'move':
-			return $this->do_move();
+				return $this->doMove();
 				break;
 		}
 
@@ -65,7 +66,7 @@ HELP;
 		return -1;
 	}
 
-	protected function do_list()
+	protected function doList()
 	{
 		$rowfmt = ' %-3s | %-20s';
 		$current = StorageManager::getBackend();
@@ -92,7 +93,7 @@ HELP;
 		return 0;
 	}
 
-	protected function do_set()
+	protected function doSet()
 	{
 		if (count($this->args) !== 2) {
 			throw new CommandArgsException('Invalid arguments');
@@ -106,11 +107,15 @@ HELP;
 			return -1;
 		}
 
-		StorageManager::setBackend($class);
+		if (!StorageManager::setBackend($class)) {
+			$this->out($class . ' is not a valid backend storage class.');
+			return -1;
+		}
+
 		return 0;
 	}
 
-	protected function do_move()
+	protected function doMove()
 	{
 		$tables = null;
 		if (count($this->args) < 1 || count($this->args) > 2) {
@@ -126,7 +131,17 @@ HELP;
 		}
 
 		$current = StorageManager::getBackend();
-		$r = StorageManager::move($current, $tables);
+		$total = 0;
-		$this->out(sprintf('Moved %d files', $r));
+
+		do {
+			$moved = StorageManager::move($current, $tables, $this->getOption('n', 5000));
+			if ($moved) {
+				$this->out(date('[Y-m-d H:i:s] ') . sprintf('Moved %d files', $moved));
+			}
+
+			$total += $moved;
+		} while ($moved);
+
+		$this->out(sprintf(date('[Y-m-d H:i:s] ') . 'Moved %d files total', $total));
 	}
 }

src/Core/StorageManager.php

@@ -54,12 +54,18 @@ class StorageManager
 	 * @brief Set current storage backend class
 	 *
 	 * @param string $class Backend class name
+	 * @return bool
 	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
 	 */
 	public static function setBackend($class)
 	{
-		/// @todo Check that $class implements IStorage
+		if (!in_array('Friendica\Model\Storage\IStorage', class_implements($class))) {
+			return false;
+		}
+
 		Config::set('storage', 'class', $class);
+
+		return true;
 	}
 
 	/**
@@ -105,20 +111,20 @@ class StorageManager
 
 
 	/**
-	 * @brief Move resources to storage $dest
+	 * @brief Move up to 5000 resources to storage $dest
 	 *
 	 * Copy existing data to destination storage and delete from source.
 	 * This method cannot move to legacy in-table `data` field.
 	 *
-	 * @param string     $dest    Destination storage class name
+	 * @param string     $destination Storage class name
 	 * @param array|null $tables      Tables to look in for resources. Optional, defaults to ['photo', 'attach']
-	 *
+	 * @param int        $limit       Limit of the process batch size, defaults to 5000
-	 * @throws \Exception
 	 * @return int Number of moved resources
+	 * @throws \Exception
 	 */
-	public static function move($dest, $tables = null)
+	public static function move($destination, $tables = null, $limit = 5000)
 	{
-		if (is_null($dest) || empty($dest)) {
+		if (empty($destination)) {
 			throw new \Exception('Can\'t move to NULL storage backend');
 		}
 		
@@ -129,35 +135,33 @@ class StorageManager
 		$moved = 0;
 		foreach ($tables as $table) {
 			// Get the rows where backend class is not the destination backend class
-			$rr = DBA::select(
+			$resources = DBA::select(
 				$table, 
 				['id', 'data', 'backend-class', 'backend-ref'],
-				['`backend-class` IS NULL or `backend-class` != ?' , $dest ]
+				['`backend-class` IS NULL or `backend-class` != ?', $destination],
+				['limit' => $limit]
 			);
 
-			if (DBA::isResult($rr)) {
+			while ($resource = DBA::fetch($resources)) {
-				while($r = DBA::fetch($rr)) {
+				$id = $resource['id'];
-					$id = $r['id'];
+				$data = $resource['data'];
-					$data = $r['data'];
 				/** @var IStorage $backendClass */
-					$backendClass = $r['backend-class'];
+				$backendClass = $resource['backend-class'];
-					$backendRef = $r['backend-ref'];
+				$backendRef = $resource['backend-ref'];
-					if (!is_null($backendClass) && $backendClass !== '') {
+				if (!empty($backendClass)) {
 					Logger::log("get data from old backend " . $backendClass . " : " . $backendRef);
 					$data = $backendClass::get($backendRef);
 				}
 
-					Logger::log("save data to new backend " . $dest);
+				Logger::log("save data to new backend " . $destination);
-					/** @var IStorage $dest */
+				/** @var IStorage $destination */
-					$ref = $dest::put($data);
+				$ref = $destination::put($data);
 				Logger::log("saved data as " . $ref);
 
 				if ($ref !== '') {
 					Logger::log("update row");
-						$ru = DBA::update($table, ['backend-class' => $dest, 'backend-ref' => $ref, 'data' => ''], ['id' => $id]);
+					if (DBA::update($table, ['backend-class' => $destination, 'backend-ref' => $ref, 'data' => ''], ['id' => $id])) {
-						
+						if (!empty($backendClass)) {
-						if ($ru) {
-							if (!is_null($backendClass) && $backendClass !== '') {
 							Logger::log("delete data from old backend " . $backendClass . " : " . $backendRef);
 							$backendClass::delete($backendRef);
 						}
@@ -165,7 +169,8 @@ class StorageManager
 					}
 				}
 			}
-			}
+
+			DBA::close($resources);
 		}
 
 		return $moved;

src/Database/DBA.php

@@ -1484,7 +1484,7 @@ class DBA
 		}
 
 		$limit_string = '';
-		if (isset($params['limit']) && is_int($params['limit'])) {
+		if (isset($params['limit']) && is_numeric($params['limit'])) {
 			$limit_string = " LIMIT " . intval($params['limit']);
 		}
 

src/Worker/CronJobs.php

@@ -35,54 +35,42 @@ class CronJobs
 
 		Logger::log("Starting cronjob " . $command, Logger::DEBUG);
 
-		// Call possible post update functions
+		switch($command) {
-		// see src/Database/PostUpdate.php for more details
+			case 'post_update':
-		if ($command == 'post_update') {
 				PostUpdate::update();
-			return;
+				break;
-		}
 
-		// update nodeinfo data
+			case 'nodeinfo':
-		if ($command == 'nodeinfo') {
 				nodeinfo_cron();
-			return;
+				break;
-		}
 
-		// Expire and remove user entries
+			case 'expire_and_remove_users':
-		if ($command == 'expire_and_remove_users') {
 				self::expireAndRemoveUsers();
-			return;
+				break;
-		}
 
-		if ($command == 'update_contact_birthdays') {
+			case 'update_contact_birthdays':
 				Contact::updateBirthdays();
-			return;
+				break;
-		}
 
-		if ($command == 'update_photo_albums') {
+			case 'update_photo_albums':
 				self::updatePhotoAlbums();
-			return;
+				break;
-		}
 
-		// Clear cache entries
+			case 'clear_cache':
-		if ($command == 'clear_cache') {
 				self::clearCache($a);
-			return;
+				break;
-		}
 
-		// Repair missing Diaspora values in contacts
+			case 'repair_diaspora':
-		if ($command == 'repair_diaspora') {
 				self::repairDiaspora($a);
-			return;
+				break;
-		}
 
-		// Repair entries in the database
+			case 'repair_database':
-		if ($command == 'repair_database') {
 				self::repairDatabase();
-			return;
+				break;
-		}
 
+			default:
 				Logger::log("Xronjob " . $command . " is unknown.", Logger::DEBUG);
+		}
 
 		return;
 	}