Merge pull request #22269 from owncloud/issue-22243-avoid-deadlock-with-lots-of-entries-to-cleanup

Chunk the cleanup queries to make sure they don't time out

apps/files/command/deleteorphanedfiles.php

@@ -33,6 +33,8 @@ use Symfony\Component\Console\Output\OutputInterface;
  */
 class DeleteOrphanedFiles extends Command {
 
+	const CHUNK_SIZE = 200;
+
 	/**
 	 * @var IDBConnection
 	 */
@@ -50,13 +52,31 @@ class DeleteOrphanedFiles extends Command {
 	}
 
 	public function execute(InputInterface $input, OutputInterface $output) {
+		$deletedEntries = 0;
 
-		$sql =
+		$query = $this->connection->getQueryBuilder();
-			'DELETE FROM `*PREFIX*filecache` ' .
+		$query->select('fc.fileid')
-			'WHERE NOT EXISTS ' .
+			->from('filecache', 'fc')
-			'(SELECT 1 FROM `*PREFIX*storages` WHERE `storage` = `numeric_id`)';
+			->where($query->expr()->isNull('s.numeric_id'))
+			->leftJoin('fc', 'storages', 's', $query->expr()->eq('fc.storage', 's.numeric_id'))
+			->setMaxResults(self::CHUNK_SIZE);
+
+		$deleteQuery = $this->connection->getQueryBuilder();
+		$deleteQuery->delete('filecache')
+			->where($deleteQuery->expr()->eq('fileid', $deleteQuery->createParameter('objectid')));
+
+		$deletedInLastChunk = self::CHUNK_SIZE;
+		while ($deletedInLastChunk === self::CHUNK_SIZE) {
+			$deletedInLastChunk = 0;
+			$result = $query->execute();
+			while ($row = $result->fetch()) {
+				$deletedInLastChunk++;
+				$deletedEntries += $deleteQuery->setParameter('objectid', (int) $row['fileid'])
+					->execute();
+			}
+			$result->closeCursor();
+		}
 
-		$deletedEntries = $this->connection->executeUpdate($sql);
 		$output->writeln("$deletedEntries orphaned file cache entries deleted");
 	}
 

apps/files/lib/backgroundjob/deleteorphaneditems.php

@@ -29,6 +29,8 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
  */
 class DeleteOrphanedItems extends TimedJob {
 
+	const CHUNK_SIZE = 200;
+
 	/** @var \OCP\IDBConnection */
 	protected $connection;
 
@@ -66,19 +68,38 @@ class DeleteOrphanedItems extends TimedJob {
 	/**
 	 * Deleting orphaned system tag mappings
 	 *
+	 * @param string $table
+	 * @param string $idCol
+	 * @param string $typeCol
 	 * @return int Number of deleted entries
 	 */
 	protected function cleanUp($table, $idCol, $typeCol) {
-		$subQuery = $this->connection->getQueryBuilder();
+		$deletedEntries = 0;
-		$subQuery->select($subQuery->expr()->literal('1'))
-			->from('filecache', 'f')
-			->where($subQuery->expr()->eq($idCol, 'f.fileid'));
 
 		$query = $this->connection->getQueryBuilder();
-		$deletedEntries = $query->delete($table)
+		$query->select('t1.' . $idCol)
+			->from($table, 't1')
 			->where($query->expr()->eq($typeCol, $query->expr()->literal('files')))
-			->andWhere($query->expr()->isNull($query->createFunction('(' . $subQuery->getSql() . ')')))
+			->andWhere($query->expr()->isNull('t2.fileid'))
+			->leftJoin('t1', 'filecache', 't2', $query->expr()->eq($query->expr()->castColumn('t1.' . $idCol, IQueryBuilder::PARAM_INT), 't2.fileid'))
+			->groupBy('t1.' . $idCol)
+			->setMaxResults(self::CHUNK_SIZE);
+
+		$deleteQuery = $this->connection->getQueryBuilder();
+		$deleteQuery->delete($table)
+			->where($deleteQuery->expr()->eq($idCol, $deleteQuery->createParameter('objectid')));
+
+		$deletedInLastChunk = self::CHUNK_SIZE;
+		while ($deletedInLastChunk === self::CHUNK_SIZE) {
+			$result = $query->execute();
+			$deletedInLastChunk = 0;
+			while ($row = $result->fetch()) {
+				$deletedInLastChunk++;
+				$deletedEntries += $deleteQuery->setParameter('objectid', (int) $row[$idCol])
 					->execute();
+			}
+			$result->closeCursor();
+		}
 
 		return $deletedEntries;
 	}
@@ -111,8 +132,7 @@ class DeleteOrphanedItems extends TimedJob {
 	 * @return int Number of deleted entries
 	 */
 	protected function cleanComments() {
-		$qb = $this->connection->getQueryBuilder();
+		$deletedEntries = $this->cleanUp('comments', 'object_id', 'object_type');
-		$deletedEntries = $this->cleanUp('comments', $qb->expr()->castColumn('object_id', IQueryBuilder::PARAM_INT), 'object_type');
 		$this->logger->debug("$deletedEntries orphaned comments deleted", ['app' => 'DeleteOrphanedItems']);
 		return $deletedEntries;
 	}
@@ -123,8 +143,7 @@ class DeleteOrphanedItems extends TimedJob {
 	 * @return int Number of deleted entries
 	 */
 	protected function cleanCommentMarkers() {
-		$qb = $this->connection->getQueryBuilder();
+		$deletedEntries = $this->cleanUp('comments_read_markers', 'object_id', 'object_type');
-		$deletedEntries = $this->cleanUp('comments_read_markers', $qb->expr()->castColumn('object_id', IQueryBuilder::PARAM_INT), 'object_type');
 		$this->logger->debug("$deletedEntries orphaned comment read marks deleted", ['app' => 'DeleteOrphanedItems']);
 		return $deletedEntries;
 	}

lib/private/repair/repairinvalidshares.php

@@ -30,6 +30,8 @@ use OC\Hooks\BasicEmitter;
  */
 class RepairInvalidShares extends BasicEmitter implements \OC\RepairStep {
 
+	const CHUNK_SIZE = 200;
+
 	/**
 	 * @var \OCP\IConfig
 	 */
@@ -83,18 +85,24 @@ class RepairInvalidShares extends BasicEmitter implements \OC\RepairStep {
 			->where($query->expr()->isNotNull('s1.parent'))
 				->andWhere($query->expr()->isNull('s2.id'))
 			->leftJoin('s1', 'share', 's2', $query->expr()->eq('s1.parent', 's2.id'))
-			->groupBy('s1.parent');
+			->groupBy('s1.parent')
+			->setMaxResults(self::CHUNK_SIZE);
 
 		$deleteQuery = $this->connection->getQueryBuilder();
 		$deleteQuery->delete('share')
-			->where($query->expr()->eq('parent', $deleteQuery->createParameter('parent')));
+			->where($deleteQuery->expr()->eq('parent', $deleteQuery->createParameter('parent')));
 
+		$deletedInLastChunk = self::CHUNK_SIZE;
+		while ($deletedInLastChunk === self::CHUNK_SIZE) {
+			$deletedInLastChunk = 0;
 			$result = $query->execute();
 			while ($row = $result->fetch()) {
+				$deletedInLastChunk++;
 				$deletedEntries += $deleteQuery->setParameter('parent', (int) $row['parent'])
 					->execute();
 			}
 			$result->closeCursor();
+		}
 
 		if ($deletedEntries) {
 			$this->emit('\OC\Repair', 'info', array('Removed ' . $deletedEntries . ' shares where the parent did not exist'));