Merge pull request #14211 from annando/lock-optimize

Lock for "optimize" / expire in chunks

src/DI.php

@@ -66,7 +66,7 @@ abstract class DI
 	public static function setCompositeRootDependencyByHand()
 	{
 		$database = static::dba();
-		$database->setDependency(static::config(), static::profiler(), static::logger());
+		$database->setDependency(static::config(), static::profiler(), static::logger(), static::lock());
 	}
 
 	/**

src/Database/DBA.php

@@ -21,6 +21,7 @@
 
 namespace Friendica\Database;
 
+use Friendica\Core\Lock\Exception\LockPersistenceException;
 use Friendica\DI;
 use mysqli;
 use mysqli_result;
@@ -823,6 +824,27 @@ class DBA
 		return DI::dba()->optimizeTable($table);
 	}
 
+	/**
+	 * Acquire a lock to prevent a table optimization
+	 *
+	 * @return bool 
+	 * @throws LockPersistenceException 
+	 */
+	public static function acquireOptimizeLock(): bool
+	{
+		return DI::dba()->acquireOptimizeLock();
+	}
+
+	/**
+	 * Release the table optimization lock
+	 * @return bool 
+	 * @throws LockPersistenceException 
+	 */
+	public static function releaseOptimizeLock(): bool
+	{
+		return DI::dba()->releaseOptimizeLock();
+	}
+
 	/**
 	 * Kill sleeping database processes
 	 */

src/Database/Database.php

@@ -22,6 +22,8 @@
 namespace Friendica\Database;
 
 use Friendica\Core\Config\Capability\IManageConfigValues;
+use Friendica\Core\Lock\Capability\ICanLock;
+use Friendica\Core\Lock\Exception\LockPersistenceException;
 use Friendica\Core\System;
 use Friendica\Database\Definition\DbaDefinition;
 use Friendica\Database\Definition\ViewDefinition;
@@ -50,6 +52,8 @@ class Database
 	const INSERT_UPDATE  = 1;
 	const INSERT_IGNORE  = 2;
 
+	const LOCK_OPTIMIZE = 'database::optimize_tables';
+
 	protected $connected = false;
 
 	/**
@@ -64,6 +68,11 @@ class Database
 	 * @var LoggerInterface
 	 */
 	protected $logger = null;
+	/**
+	 * @var ICanLock
+	 */
+	protected $syslock = null;
+
 	protected $server_info = '';
 	/** @var PDO|mysqli */
 	protected $connection;
@@ -106,11 +115,12 @@ class Database
 	 *
 	 * @todo Make this method obsolete - use a clean pattern instead ...
 	 */
-	public function setDependency(IManageConfigValues $config, Profiler $profiler, LoggerInterface $logger)
+	public function setDependency(IManageConfigValues $config, Profiler $profiler, LoggerInterface $logger, ICanLock $lock)
 	{
 		$this->logger   = $logger;
 		$this->profiler = $profiler;
 		$this->config   = $config;
+		$this->syslock  = $lock;
 	}
 
 	/**
@@ -1755,7 +1765,42 @@ class Database
 	 */
 	public function optimizeTable(string $table): bool
 	{
-		return $this->e("OPTIMIZE TABLE " . DBA::buildTableString([$table])) !== false;
+		if ($this->syslock->isLocked(self::LOCK_OPTIMIZE)) {
+			$this->logger->info('Optimization is locked');
+			return false;
+		}
+
+		if (!$this->acquireOptimizeLock()) {
+			return false;
+		}
+
+		$result = $this->e("OPTIMIZE TABLE " . DBA::buildTableString([$table])) !== false;
+
+		$this->releaseOptimizeLock();
+
+		return $result;
+	}
+
+	/**
+	 * Acquire a lock to prevent a table optimization
+	 *
+	 * @return bool 
+	 * @throws LockPersistenceException 
+	 */
+	public function acquireOptimizeLock(): bool
+	{
+		return $this->syslock->acquire(self::LOCK_OPTIMIZE, 0);
+	}
+
+	/**
+	 * Release the table optimization lock
+	 *
+	 * @return bool 
+	 * @throws LockPersistenceException 
+	 */
+	public function releaseOptimizeLock(): bool
+	{
+		return $this->syslock->release(self::LOCK_OPTIMIZE);
 	}
 
 	/**

src/Worker/ExpirePosts.php

@@ -40,6 +40,11 @@ class ExpirePosts
 	 */
 	public static function execute()
 	{
+		if (!DBA::acquireOptimizeLock()) {
+			Logger::warning('Lock could not be acquired');
+			return;
+		}
+
 		self::deleteExpiredOriginPosts();
 
 		self::deleteOrphanedEntries();
@@ -52,6 +57,8 @@ class ExpirePosts
 			self::addMissingEntries();
 		}
 
+		DBA::releaseOptimizeLock();
+
 		// Set the expiry for origin posts
 		Worker::add(Worker::PRIORITY_LOW, 'Expire');
 
@@ -69,15 +76,22 @@ class ExpirePosts
 		Logger::notice('Delete expired posts');
 		// physically remove anything that has been deleted for more than two months
 		$condition = ["`gravity` = ? AND `deleted` AND `changed` < ?", Item::GRAVITY_PARENT, DateTimeFormat::utc('now - 60 days')];
-		$rows = Post::select(['guid', 'uri-id', 'uid'],  $condition);
+		$pass = 0;
+		do {
+			++$pass;
+			$rows = Post::select(['guid', 'uri-id', 'uid'],  $condition, ['limit' => 100]);
+			$affected_count = 0;
 			while ($row = Post::fetch($rows)) {
-			Logger::info('Delete expired item', ['uri-id' => $row['uri-id'], 'guid' => $row['guid']]);
+				Logger::info('Delete expired item', ['pass' => $pass, 'uri-id' => $row['uri-id'], 'guid' => $row['guid']]);
 				Post\User::delete(['parent-uri-id' => $row['uri-id'], 'uid' => $row['uid']]);
+				$affected_count += DBA::affectedRows();
 				Post\Origin::delete(['parent-uri-id' => $row['uri-id'], 'uid' => $row['uid']]);
+				$affected_count += DBA::affectedRows();
 			}
 			DBA::close($rows);
-
+			DBA::commit();
-		Logger::notice('Delete expired posts - done');
+			Logger::notice('Delete expired posts - done', ['pass' => $pass, 'rows' => $affected_count]);
+		} while ($affected_count);
 	}
 
 	/**
@@ -106,6 +120,7 @@ class ExpirePosts
 				$affected_count += DBA::affectedRows();
 			}
 			DBA::close($uris);
+			DBA::commit();
 			Logger::notice('Orphaned entries deleted', ['table' => $table, 'rows' => $affected_count]);
 		}
 		Logger::notice('Delete orphaned entries - done');
@@ -172,14 +187,18 @@ class ExpirePosts
 	{
 		// We have to avoid deleting newly created "item-uri" entries.
 		// So we fetch a post that had been stored yesterday and only delete older ones.
-		$item = Post::selectFirstThread(['uri-id'], ["`uid` = ? AND `received` < ?", 0, DateTimeFormat::utc('now - 1 day')],
+		$item = Post::selectFirstThread(
-			['order' => ['received' => true]]);
+			['uri-id'],
+			["`uid` = ? AND `received` < ?", 0, DateTimeFormat::utc('now - 1 day')],
+			['order' => ['received' => true]]
+		);
 		if (empty($item['uri-id'])) {
 			Logger::warning('No item with uri-id found - we better quit here');
 			return;
 		}
 		Logger::notice('Start collecting orphaned URI-ID', ['last-id' => $item['uri-id']]);
-		$uris = DBA::select('item-uri', ['id'], ["`id` < ?
+		$condition = [
+			"`id` < ?
 			AND NOT EXISTS(SELECT `uri-id` FROM `post-user` WHERE `uri-id` = `item-uri`.`id`)
 			AND NOT EXISTS(SELECT `parent-uri-id` FROM `post-user` WHERE `parent-uri-id` = `item-uri`.`id`)
 			AND NOT EXISTS(SELECT `thr-parent-id` FROM `post-user` WHERE `thr-parent-id` = `item-uri`.`id`)
@@ -195,18 +214,24 @@ class ExpirePosts
 			AND NOT EXISTS(SELECT `uri-id` FROM `post-delivery` WHERE `uri-id` = `item-uri`.`id`)
 			AND NOT EXISTS(SELECT `uri-id` FROM `post-delivery` WHERE `inbox-id` = `item-uri`.`id`)
 			AND NOT EXISTS(SELECT `parent-uri-id` FROM `mail` WHERE `parent-uri-id` = `item-uri`.`id`)
-			AND NOT EXISTS(SELECT `thr-parent-id` FROM `mail` WHERE `thr-parent-id` = `item-uri`.`id`)", $item['uri-id']]);
+			AND NOT EXISTS(SELECT `thr-parent-id` FROM `mail` WHERE `thr-parent-id` = `item-uri`.`id`)", $item['uri-id']
-
+		];
-		Logger::notice('Start deleting orphaned URI-ID', ['last-id' => $item['uri-id']]);
+		$pass = 0;
+		do {
+			++$pass;
+			$uris = DBA::select('item-uri', ['id'], $condition, ['limit' => 1000]);
+			Logger::notice('Start deleting orphaned URI-ID', ['pass' => $pass, 'last-id' => $item['uri-id']]);
 			$affected_count = 0;
 			while ($rows = DBA::toArray($uris, false, 100)) {
 				$ids = array_column($rows, 'id');
 				DBA::delete('item-uri', ['id' => $ids]);
 				$affected_count += DBA::affectedRows();
-			Logger::info('Deleted', ['rows' => $affected_count]);
+				Logger::info('Deleted', ['pass' => $pass, 'rows' => $affected_count]);
 			}
 			DBA::close($uris);
-		Logger::notice('Orphaned URI-ID entries removed', ['rows' => $affected_count]);
+			DBA::commit();
+			Logger::notice('Orphaned URI-ID entries removed', ['pass' => $pass, 'rows' => $affected_count]);
+		} while ($affected_count);
 	}
 
 	/**
@@ -227,8 +252,8 @@ class ExpirePosts
 
 		if (!empty($expire_days)) {
 			Logger::notice('Start collecting expired threads', ['expiry_days' => $expire_days]);
-			$uris = DBA::select('item-uri', ['id'], ["`id` IN
+			$condition = [
-				(SELECT `uri-id` FROM `post-thread` WHERE `received` < ?
+				"`id` IN (SELECT `uri-id` FROM `post-thread` WHERE `received` < ?
 				AND NOT `uri-id` IN (SELECT `uri-id` FROM `post-thread-user`
 					WHERE (`mention` OR `starred` OR `wall`) AND `uri-id` = `post-thread`.`uri-id`)
 				AND NOT `uri-id` IN (SELECT `uri-id` FROM `post-category`
@@ -243,9 +268,14 @@ class ExpirePosts
 					WHERE (`origin` OR `event-id` != 0 OR `post-type` = ?) AND `parent-uri-id` = `post-thread`.`uri-id`)
 				AND NOT `uri-id` IN (SELECT `uri-id` FROM `post-content`
 					WHERE `resource-id` != 0 AND `uri-id` = `post-thread`.`uri-id`))",
-			    DateTimeFormat::utc('now - ' . (int)$expire_days . ' days'), Item::PT_PERSONAL_NOTE]);
+				DateTimeFormat::utc('now - ' . (int)$expire_days . ' days'), Item::PT_PERSONAL_NOTE
+			];
+			$pass = 0;
+			do {
+				++$pass;
+				$uris = DBA::select('item-uri', ['id'], $condition, ['limit' => 100]);
 
-			Logger::notice('Start deleting expired threads');
+				Logger::notice('Start deleting expired threads', ['pass' => $pass]);
 				$affected_count = 0;
 				while ($rows = DBA::toArray($uris, false, 100)) {
 					$ids = array_column($rows, 'id');
@@ -253,21 +283,27 @@ class ExpirePosts
 					$affected_count += DBA::affectedRows();
 				}
 				DBA::close($uris);
-
+				DBA::commit();
-			Logger::notice('Deleted expired threads', ['rows' => $affected_count]);
+				Logger::notice('Deleted expired threads', ['pass' => $pass, 'rows' => $affected_count]);
+			} while ($affected_count);
 		}
 
 		if (!empty($expire_days_unclaimed)) {
 			Logger::notice('Start collecting unclaimed public items', ['expiry_days' => $expire_days_unclaimed]);
-			$uris = DBA::select('item-uri', ['id'], ["`id` IN
+			$condition = [
-				(SELECT `uri-id` FROM `post-user` WHERE `gravity` = ? AND `uid` = ? AND `received` < ?
+				"`id` IN (SELECT `uri-id` FROM `post-user` WHERE `gravity` = ? AND `uid` = ? AND `received` < ?
 				AND NOT `uri-id` IN (SELECT `parent-uri-id` FROM `post-user` AS `i` WHERE `i`.`uid` != ?
 					AND `i`.`parent-uri-id` = `post-user`.`uri-id`)
 				AND NOT `uri-id` IN (SELECT `parent-uri-id` FROM `post-user` AS `i` WHERE `i`.`uid` = ?
 					AND `i`.`parent-uri-id` = `post-user`.`uri-id` AND `i`.`received` > ?))",
-				Item::GRAVITY_PARENT, 0, DateTimeFormat::utc('now - ' . (int)$expire_days_unclaimed . ' days'), 0, 0, DateTimeFormat::utc('now - ' . (int)$expire_days_unclaimed . ' days')]);
+				Item::GRAVITY_PARENT, 0, DateTimeFormat::utc('now - ' . (int)$expire_days_unclaimed . ' days'), 0, 0, DateTimeFormat::utc('now - ' . (int)$expire_days_unclaimed . ' days')
+			];
+			$pass = 0;
+			do {
+				++$pass;
+				$uris = DBA::select('item-uri', ['id'], $condition, ['limit' => 100]);
 
-			Logger::notice('Start deleting unclaimed public items');
+				Logger::notice('Start deleting unclaimed public items', ['pass' => $pass]);
 				$affected_count = 0;
 				while ($rows = DBA::toArray($uris, false, 100)) {
 					$ids = array_column($rows, 'id');
@@ -275,7 +311,9 @@ class ExpirePosts
 					$affected_count += DBA::affectedRows();
 				}
 				DBA::close($uris);
-			Logger::notice('Deleted unclaimed public items', ['rows' => $affected_count]);
+				DBA::commit();
+				Logger::notice('Deleted unclaimed public items', ['pass' => $pass, 'rows' => $affected_count]);
+			} while ($affected_count);
 		}
 	}
 }