From 138219d74a2a98f9fcc3e875658c3b6c259340d9 Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Fri, 4 Mar 2016 15:42:35 +0100
Subject: [PATCH] Run cleanup of expired DB file locks to background job

* fixes #22819

The old way fired a DELETE statement on each destruction of the
DBLockingProvider. Which could cause a lot of queries. It's enough
to run this every 5 minutes in a background job, which in the end
could result in file locks that exists 5 minutes longer - in the
worst case and for not properly released locks.

This makes the DB based locking a lot more performant and could
result in a similar performance to the Redis based locking provider.
---
 apps/files/appinfo/info.xml                   |  2 +-
 apps/files/appinfo/install.php                |  1 +
 apps/files/appinfo/update.php                 |  1 +
 .../lib/backgroundjob/cleanupfilelocks.php    | 57 +++++++++++++++++++
 lib/private/lock/dblockingprovider.php        | 26 ++++-----
 5 files changed, 71 insertions(+), 16 deletions(-)
 create mode 100644 apps/files/lib/backgroundjob/cleanupfilelocks.php

diff --git a/apps/files/appinfo/info.xml b/apps/files/appinfo/info.xml
index dc9d6b3c21..79dc4b3134 100644
--- a/apps/files/appinfo/info.xml
+++ b/apps/files/appinfo/info.xml
@@ -6,7 +6,7 @@
 	<licence>AGPL</licence>
 	<author>Robin Appelman, Vincent Petry</author>
 	<default_enable/>
-	<version>1.5.0</version>
+	<version>1.5.1</version>
 	<types>
 		<filesystem/>
 	</types>
diff --git a/apps/files/appinfo/install.php b/apps/files/appinfo/install.php
index 4d1e59d214..55514935cd 100644
--- a/apps/files/appinfo/install.php
+++ b/apps/files/appinfo/install.php
@@ -24,3 +24,4 @@
 // Cron job for scanning user storages
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\ScanFiles');
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedItems');
+\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\CleanupFileLocks');
diff --git a/apps/files/appinfo/update.php b/apps/files/appinfo/update.php
index e4f8f8f29d..72aff4cd0d 100644
--- a/apps/files/appinfo/update.php
+++ b/apps/files/appinfo/update.php
@@ -102,3 +102,4 @@ if ($installedVersion === '1.1.9' && (
 // Add cron job for scanning user storages
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\ScanFiles');
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedItems');
+\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\CleanupFileLocks');
diff --git a/apps/files/lib/backgroundjob/cleanupfilelocks.php b/apps/files/lib/backgroundjob/cleanupfilelocks.php
new file mode 100644
index 0000000000..b5cf8e9455
--- /dev/null
+++ b/apps/files/lib/backgroundjob/cleanupfilelocks.php
@@ -0,0 +1,57 @@
+<?php
+/**
+ * @author Morris Jobke <hey@morrisjobke.de>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Files\BackgroundJob;
+
+use OC\BackgroundJob\TimedJob;
+use OC\Lock\DBLockingProvider;
+
+/**
+ * Clean up all file locks that are expired for the DB file locking provider
+ */
+class CleanupFileLocks extends TimedJob {
+
+	/**
+	 * Default interval in minutes
+	 *
+	 * @var int $defaultIntervalMin
+	 **/
+	protected $defaultIntervalMin = 5;
+
+	/**
+	 * sets the correct interval for this timed job
+	 */
+	public function __construct() {
+		$this->interval = $this->defaultIntervalMin * 60;
+	}
+
+	/**
+	 * Makes the background job do its work
+	 *
+	 * @param array $argument unused argument
+	 */
+	public function run($argument) {
+		$lockingProvider = \OC::$server->getLockingProvider();
+		if($lockingProvider instanceof DBLockingProvider) {
+			$lockingProvider->cleanExpiredLocks();
+		}
+	}
+}
diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php
index 647250cdb6..c10cd8636a 100644
--- a/lib/private/lock/dblockingprovider.php
+++ b/lib/private/lock/dblockingprovider.php
@@ -235,10 +235,17 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 */
 	public function cleanExpiredLocks() {
 		$expire = $this->timeFactory->getTime();
-		$this->connection->executeUpdate(
-			'DELETE FROM `*PREFIX*file_locks` WHERE `ttl` < ?',
-			[$expire]
-		);
+		try {
+			$this->connection->executeUpdate(
+				'DELETE FROM `*PREFIX*file_locks` WHERE `ttl` < ?',
+				[$expire]
+			);
+		} catch (\Exception $e) {
+			// If the table is missing, the clean up was successful
+			if ($this->connection->tableExists('file_locks')) {
+				throw $e;
+			}
+		}
 	}
 
 	/**
@@ -257,15 +264,4 @@ class DBLockingProvider extends AbstractLockingProvider {
 			}
 		}
 	}
-
-	public function __destruct() {
-		try {
-			$this->cleanExpiredLocks();
-		} catch (\Exception $e) {
-			// If the table is missing, the clean up was successful
-			if ($this->connection->tableExists('file_locks')) {
-				throw $e;
-			}
-		}
-	}
 }