aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLouis Chemineau <louis@chmn.me>2025-05-21 11:55:45 +0200
committerLouis Chemineau <louis@chmn.me>2025-05-21 11:55:45 +0200
commit08928d572a628d00e9eb23dbd15ffbf0c6fa2c62 (patch)
tree2246161b0eb11af13ec064de4738bed12a96cfea
parentea0f589c327ef0cbf2e114265fcd5898a203605a (diff)
downloadnextcloud-server-backport/52825/stable29.tar.gz
nextcloud-server-backport/52825/stable29.zip
feat(files_trashbin): Refactor expire background job to support parallel runbackport/52825/stable29
- Follow-up of https://github.com/nextcloud/server/pull/51600 The original PR introduced the possibility to continue an `ExpireTrash` job by saving the offset. This was to prevent having to start over the whole user list when the job crashed or was killed. But on big instances, one process is not enough to go through all the users in a timely manner. Supporting parallel run allows covering more ground faster. This PR introduced this possibility. We are now storing the offset right away to allow another parallel job to pick up the task at that point. We are arbitrarily cutting the user list in chunk of 10 to not drastically overflow the 30 minutes time limit. Signed-off-by: Louis Chemineau <louis@chmn.me>
-rw-r--r--apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php59
-rw-r--r--apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php9
2 files changed, 40 insertions, 28 deletions
diff --git a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php
index a12c8530702..4038c7c7b34 100644
--- a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php
+++ b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php
@@ -26,27 +26,31 @@
*/
namespace OCA\Files_Trashbin\BackgroundJob;
+use OC\Files\SetupManager;
+use OC\Files\View;
use OCA\Files_Trashbin\Expiration;
use OCA\Files_Trashbin\Helper;
use OCA\Files_Trashbin\Trashbin;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;
use OCP\IAppConfig;
+use OCP\IUser;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;
class ExpireTrash extends TimedJob {
+ private const THIRTY_MINUTES = 30 * 60;
public function __construct(
private IAppConfig $appConfig,
private IUserManager $userManager,
private Expiration $expiration,
private LoggerInterface $logger,
+ private SetupManager $setupManager,
ITimeFactory $time
) {
parent::__construct($time);
- // Run once per 30 minutes
- $this->setInterval(60 * 30);
+ $this->setInterval(self::THIRTY_MINUTES);
}
protected function run($argument) {
@@ -60,44 +64,47 @@ class ExpireTrash extends TimedJob {
return;
}
- $stopTime = time() + 60 * 30; // Stops after 30 minutes.
- $offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
- $users = $this->userManager->getSeenUsers($offset);
+ $stopTime = time() + self::THIRTY_MINUTES;
- foreach ($users as $user) {
- try {
+ do {
+ $this->appConfig->clearCache();
+ $offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
+ $this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset + 10);
+
+ $users = $this->userManager->getSeenUsers($offset, 10);
+ $count = 0;
+
+ foreach ($users as $user) {
$uid = $user->getUID();
- if (!$this->setupFS($uid)) {
- continue;
+ $count++;
+
+ try {
+ if ($this->setupFS($user)) {
+ $dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
+ Trashbin::deleteExpiredFiles($dirContent, $uid);
+ }
+ } catch (\Throwable $e) {
+ $this->logger->error('Error while expiring trashbin for user ' . $uid, ['exception' => $e]);
+ } finally {
+ $this->setupManager->tearDown();
}
- $dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
- Trashbin::deleteExpiredFiles($dirContent, $uid);
- } catch (\Throwable $e) {
- $this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
}
- $offset++;
+ } while (time() < $stopTime && $count === 10);
- if ($stopTime < time()) {
- $this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset);
- \OC_Util::tearDownFS();
- return;
- }
+ if ($count < 10) {
+ $this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
}
-
- $this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
- \OC_Util::tearDownFS();
}
/**
* Act on behalf on trash item owner
*/
- protected function setupFS(string $user): bool {
- \OC_Util::tearDownFS();
- \OC_Util::setupFS($user);
+ protected function setupFS(IUser $user): bool {
+ $this->setupManager->setupForUser($user);
// Check if this user has a trashbin directory
- $view = new \OC\Files\View('/' . $user);
+ $view = new View('/' . $user->getUID());
if (!$view->is_dir('/files_trashbin/files')) {
return false;
}
diff --git a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php
index 56deb757c36..50c5950eadf 100644
--- a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php
+++ b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php
@@ -25,6 +25,7 @@
namespace OCA\Files_Trashbin\Tests\BackgroundJob;
+use OC\Files\SetupManager;
use OCA\Files_Trashbin\BackgroundJob\ExpireTrash;
use OCA\Files_Trashbin\Expiration;
use OCP\AppFramework\Utility\ITimeFactory;
@@ -54,6 +55,9 @@ class ExpireTrashTest extends TestCase {
/** @var ITimeFactory&MockObject */
private $time;
+ /** @var SetupManager&MockObject */
+ private $setupManager;
+
protected function setUp(): void {
parent::setUp();
@@ -62,6 +66,7 @@ class ExpireTrashTest extends TestCase {
$this->expiration = $this->createMock(Expiration::class);
$this->jobList = $this->createMock(IJobList::class);
$this->logger = $this->createMock(LoggerInterface::class);
+ $this->setupManager = $this->createMock(SetupManager::class);
$this->time = $this->createMock(ITimeFactory::class);
$this->time->method('getTime')
@@ -81,7 +86,7 @@ class ExpireTrashTest extends TestCase {
->with('files_trashbin', 'background_job_expire_trash_offset', 0)
->willReturn(0);
- $job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
+ $job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->setupManager, $this->time);
$job->start($this->jobList);
}
@@ -92,7 +97,7 @@ class ExpireTrashTest extends TestCase {
$this->expiration->expects($this->never())
->method('getMaxAgeAsTimestamp');
- $job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
+ $job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->setupManager, $this->time);
$job->start($this->jobList);
}
}