Skip to content

Commit f996f08

Browse files
artongeartonge
committed
feat(files_trashbin): Refactor expire background job to support parallel run
- Follow-up of #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>
1 parent 1d91e40 commit f996f08

File tree

3 files changed

+76
-41
lines changed

3 files changed

+76
-41
lines changed

‎apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php‎

Lines changed: 71 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,34 @@
77
*/
88
namespace OCA\Files_Trashbin\BackgroundJob;
99

10+
use OC\Files\SetupManager;
1011
use OC\Files\View;
1112
use OCA\Files_Trashbin\Expiration;
1213
use OCA\Files_Trashbin\Helper;
1314
use OCA\Files_Trashbin\Trashbin;
1415
use OCP\AppFramework\Utility\ITimeFactory;
1516
use OCP\BackgroundJob\TimedJob;
1617
use OCP\IAppConfig;
18+
use OCP\IUser;
1719
use OCP\IUserManager;
20+
use OCP\Lock\ILockingProvider;
1821
use Psr\Log\LoggerInterface;
1922

2023
class ExpireTrash extends TimedJob {
24+
private const THIRTY_MINUTES = 30 * 60;
25+
private const USER_BATCH_SIZE = 10;
26+
2127
public function __construct(
2228
private IAppConfig $appConfig,
2329
private IUserManager $userManager,
2430
private Expiration $expiration,
2531
private LoggerInterface $logger,
32+
private SetupManager $setupManager,
33+
private ILockingProvider $lockingProvider,
2634
ITimeFactory $time,
2735
) {
2836
parent::__construct($time);
29-
// Run once per 30 minutes
30-
$this->setInterval(60 * 30);
37+
$this->setInterval(self::THIRTY_MINUTES);
3138
}
3239

3340
protected function run($argument) {
@@ -41,48 +48,86 @@ protected function run($argument) {
4148
return;
4249
}
4350

44-
$stopTime = time() + 60 * 30; // Stops after 30 minutes.
45-
$offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
46-
$users = $this->userManager->getSeenUsers($offset);
51+
$stopTime = time() + self::THIRTY_MINUTES;
4752

48-
foreach ($users as $user) {
49-
try {
53+
do {
54+
$offset = $this->getNextOffset();
55+
$users = $this->userManager->getSeenUsers($offset, self::USER_BATCH_SIZE);
56+
$count = 0;
57+
58+
foreach ($users as $user) {
5059
$uid = $user->getUID();
51-
if (!$this->setupFS($uid)) {
52-
continue;
60+
$count++;
61+
62+
try {
63+
if ($this->setupFS($user)) {
64+
$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
65+
Trashbin::deleteExpiredFiles($dirContent, $uid);
66+
}
67+
} catch (\Throwable $e) {
68+
$this->logger->error('Error while expiring trashbin for user ' . $uid, ['exception' => $e]);
69+
} finally {
70+
$this->setupManager->tearDown();
5371
}
54-
$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
55-
Trashbin::deleteExpiredFiles($dirContent, $uid);
56-
} catch (\Throwable $e) {
57-
$this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
5872
}
5973

60-
$offset++;
74+
} while (time() < $stopTime && $count === self::USER_BATCH_SIZE);
6175

62-
if ($stopTime < time()) {
63-
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset);
64-
\OC_Util::tearDownFS();
65-
return;
66-
}
76+
if ($count < self::USER_BATCH_SIZE) {
77+
$this->resetOffset();
6778
}
68-
69-
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
70-
\OC_Util::tearDownFS();
7179
}
7280

7381
/**
7482
* Act on behalf on trash item owner
7583
*/
76-
protected function setupFS(string $user): bool {
77-
\OC_Util::tearDownFS();
78-
\OC_Util::setupFS($user);
84+
protected function setupFS(IUser $user): bool {
85+
$this->setupManager->setupForUser($user);
7986

8087
// Check if this user has a trashbin directory
81-
$view = new View('/' . $user);
88+
$view = new View('/' . $user->getUID());
8289
if (!$view->is_dir('/files_trashbin/files')) {
8390
return false;
8491
}
8592

8693
return true;
8794
}
95+
96+
private function getNextOffset(): int {
97+
return $this->runMutexOperation(function() {
98+
$this->appConfig->clearCache();
99+
100+
$offset = $this->appConfig->getValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
101+
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', $offset + self::USER_BATCH_SIZE);
102+
103+
return $offset;
104+
});
105+
106+
}
107+
108+
private function resetOffset() {
109+
$this->runMutexOperation(function() {
110+
$this->appConfig->setValueInt('files_trashbin', 'background_job_expire_trash_offset', 0);
111+
});
112+
}
113+
114+
private function runMutexOperation($operation): mixed {
115+
$acquired = false;
116+
117+
while ($acquired === false) {
118+
try {
119+
$this->lockingProvider->acquireLock('background_job_expire_trash', ILockingProvider::LOCK_EXCLUSIVE, 'Expire trashbin background job');
120+
$acquired = true;
121+
} catch (\OCP\Lock\LockedException $e) {
122+
// wait a bit and try again
123+
usleep(100000);
124+
}
125+
}
126+
127+
$result = $operation();
128+
129+
$this->lockingProvider->releaseLock('background_job_expire_trash', ILockingProvider::LOCK_EXCLUSIVE);
130+
131+
return $result;
132+
}
88133
}

‎apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace OCA\Files_Trashbin\Tests\BackgroundJob;
1111

12+
use OC\Files\SetupManager;
1213
use OCA\Files_Trashbin\BackgroundJob\ExpireTrash;
1314
use OCA\Files_Trashbin\Expiration;
1415
use OCP\AppFramework\Utility\ITimeFactory;
@@ -26,6 +27,7 @@ class ExpireTrashTest extends TestCase {
2627
private IJobList&MockObject $jobList;
2728
private LoggerInterface&MockObject $logger;
2829
private ITimeFactory&MockObject $time;
30+
private SetupManager&MockObject $setupManager;
2931

3032
protected function setUp(): void {
3133
parent::setUp();
@@ -35,6 +37,7 @@ protected function setUp(): void {
3537
$this->expiration = $this->createMock(Expiration::class);
3638
$this->jobList = $this->createMock(IJobList::class);
3739
$this->logger = $this->createMock(LoggerInterface::class);
40+
$this->setupManager = $this->createMock(SetupManager::class);
3841

3942
$this->time = $this->createMock(ITimeFactory::class);
4043
$this->time->method('getTime')
@@ -54,7 +57,7 @@ public function testConstructAndRun(): void {
5457
->with('files_trashbin', 'background_job_expire_trash_offset', 0)
5558
->willReturn(0);
5659

57-
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
60+
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->setupManager, $this->time);
5861
$job->start($this->jobList);
5962
}
6063

@@ -65,7 +68,7 @@ public function testBackgroundJobDeactivated(): void {
6568
$this->expiration->expects($this->never())
6669
->method('getMaxAgeAsTimestamp');
6770

68-
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
71+
$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->setupManager, $this->time);
6972
$job->start($this->jobList);
7073
}
7174
}

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,19 +1754,6 @@
17541754
<code><![CDATA[moveMount]]></code>
17551755
</UndefinedMethod>
17561756
</file>
1757-
<file src="apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php">
1758-
<DeprecatedClass>
1759-
<code><![CDATA[\OC_Util::setupFS($user)]]></code>
1760-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1761-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1762-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1763-
</DeprecatedClass>
1764-
<DeprecatedMethod>
1765-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1766-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1767-
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
1768-
</DeprecatedMethod>
1769-
</file>
17701757
<file src="apps/files_trashbin/lib/Command/CleanUp.php">
17711758
<DeprecatedClass>
17721759
<code><![CDATA[\OC_Util::setupFS($uid)]]></code>

0 commit comments

Comments
 (0)