Skip to content

feat: Limit ExpireTrash job to 30 minutes - #51600

Merged
AndyScherzinger merged 3 commits into
masterfrom
artonge/feat/allow_partial_seen_users
Mar 30, 2025
Merged

feat: Limit ExpireTrash job to 30 minutes#51600
AndyScherzinger merged 3 commits into
masterfrom
artonge/feat/allow_partial_seen_users

Conversation

@artonge

@artongeartonge commented Mar 20, 2025•
edited
Loading

Copy link
Copy Markdown
Collaborator
  1. feat: Implements getSeenUsers to iterate over users
  2. chore: Refactor callForSeenUsers to use getSeenUsers
  3. feat: Limit ExpireTrash job to 30 minutes

@artongeartonge added enhancement 3. to review Waiting for reviews php Pull requests that update Php code labels Mar 20, 2025
@artongeartonge added this to the Nextcloud 32 milestone Mar 20, 2025
@artongeartonge self-assigned this Mar 20, 2025
@artonge
artonge requested a review from a team as a code ownerMarch 20, 2025 15:07
susnux
susnux reviewed Mar 20, 2025
View reviewed changes
Comment threadlib/private/User/Manager.php Outdated
@artonge
artonge requested a review from susnuxMarch 24, 2025 14:22
@artonge
artongeforce-pushed the artonge/feat/allow_partial_seen_users branch from ea30adf to 310945dCompareMarch 24, 2025 14:42
come-nc
come-nc approved these changes Mar 25, 2025
View reviewed changes

@come-nccome-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if we should use DateTime instead of int 🤷

Comment threadlib/public/IUserManager.php Outdated
@icewind1991

icewind1991 commented Mar 25, 2025

Copy link
Copy Markdown
Member

Instead of adding a limit to callForSeensUsers it might be better to instead have a method that returns all (seen) users as an iterator. Then the call site can do it's own limiting.

@artonge
artongeforce-pushed the artonge/feat/allow_partial_seen_users branch from 310945d to 23a5330CompareMarch 26, 2025 15:01
@artonge

artonge commented Mar 26, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Instead of adding a limit to callForSeensUsers it might be better to instead have a method that returns all (seen) users as an iterator. Then the call site can do it's own limiting.

Good idea and easy to implement :). Just pushed that version in the latest commit. Will remove the other change after review.

Note that this probably need to be backported

@artonge
artongeforce-pushed the artonge/feat/allow_partial_seen_users branch from 23a5330 to 7c5449bCompareMarch 26, 2025 15:10
@icewind1991

icewind1991 commented Mar 26, 2025

Copy link
Copy Markdown
Member

Looks good, you can also re-implement callForSeenUsers on top of getAllSeenUsers for some de-duplication

@artonge
artongeforce-pushed the artonge/feat/allow_partial_seen_users branch from 7c5449b to f5217f4CompareMarch 26, 2025 20:10
@artongeartonge changed the title feat: Allow partial callForSeenUsersfeat: Limit ExpireTrash job to 30 minutesMar 26, 2025
@artonge
artongeforce-pushed the artonge/feat/allow_partial_seen_users branch 3 times, most recently from f7ec0e9 to 700f297CompareMarch 26, 2025 20:33
@skjnldsv
skjnldsvforce-pushed the artonge/feat/allow_partial_seen_users branch from 700f297 to b7cd6fbCompareMarch 28, 2025 08:38
@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 28, 2025
backportbotBot pushed a commit that referenced this pull request May 21, 2025
@artonge@backportbot
feat(files_trashbin): Refactor expire background job to support paral…
dfd492b
…lel 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>
[skip ci]
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
75b1cd1
…lel 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>
[skip ci]
Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
3b75095
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
6b71353
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
81de5af
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
48e6568
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
496928c
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
b3a61fc
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
b96d9f5
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
209369a
…lel 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>
backportbotBot pushed a commit that referenced this pull request May 21, 2025
@artonge@backportbot
feat(files_trashbin): Refactor expire background job to support paral…
5af7009
…lel 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>
[skip ci]
backportbotBot pushed a commit that referenced this pull request May 21, 2025
@artonge@backportbot
feat(files_trashbin): Refactor expire background job to support paral…
0d622c9
…lel 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>
[skip ci]
backportbotBot pushed a commit that referenced this pull request May 21, 2025
@artonge@backportbot
feat(files_trashbin): Refactor expire background job to support paral…
6e4c5d5
…lel 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>
[skip ci]
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
3a65d5a
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
75ebc60
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
03c7c6d
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
9da31ec
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
1f2fc25
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
a58b3e9
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
82fb17f
…lel 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>
artonge added a commit that referenced this pull request May 21, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
08928d5
…lel 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>
@nextcloud-botnextcloud-bot mentioned this pull request Aug 19, 2025
Merged
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
36f2bd2
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
59590e8
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
5942a71
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
56e4db0
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
69b7391
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
8e8c280
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
cd90ee4
…lel 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>
artonge added a commit that referenced this pull request Aug 28, 2025
@artonge
feat(files_trashbin): Refactor expire background job to support paral…
c7f57a9
…lel 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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@icewind1991icewind1991icewind1991 approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@come-nccome-nccome-nc approved these changes
@AltahrimAltahrimAwaiting requested review from Altahrim
@susnuxsusnuxAwaiting requested review from susnux

Assignees

@artongeartonge

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancementphpPull requests that update Php code

Projects

None yet

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

6 participants

@artonge@icewind1991@susnux@skjnldsv@come-nc@AndyScherzinger