Skip to content

fix: make Rotate a TimedJob and run every hour - #53420

Merged
come-nc merged 1 commit into
masterfrom
fix/convert-rotate-to-timedjob
Jun 24, 2025
Merged

fix: make Rotate a TimedJob and run every hour#53420
come-nc merged 1 commit into
masterfrom
fix/convert-rotate-to-timedjob

Conversation

@salmart-dev

Copy link
Copy Markdown
Contributor

Summary

This PR is a follow-up to address issues discovered while investigating issue #49584

When running occ background-job:worker, some jobs can be seen running constantly and Rotate is one of them. It should be safe to run the rotation job every 5 seconds.

Checklist

@salmart-devsalmart-dev self-assigned this Jun 10, 2025
Comment threadlib/private/Log/Rotate.php Outdated
@salmart-devsalmart-dev added the 3. to review Waiting for reviews label Jun 10, 2025
@salmart-dev
salmart-dev marked this pull request as ready for review June 10, 2025 10:51
@salmart-dev
salmart-dev requested a review from a team as a code ownerJune 10, 2025 10:51
@salmart-dev
salmart-dev requested review from Altahrim, icewind1991 and leftybournes and removed request for a teamJune 10, 2025 10:51
@come-nccome-nc added this to the Nextcloud 32 milestone Jun 10, 2025
@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

@come-nc the job is now set to run every hour

@salmart-dev
salmart-devforce-pushed the fix/convert-rotate-to-timedjob branch from 4f529de to 25fe82dCompareJune 19, 2025 08:59
@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

In the last force-push I removed the "Merge into master" commit, which I created when clicking on the GitHub UI to update the branch instead of hitting rebase.

@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

@come-nc should I backport this to 30 and 31?

@salmart-dev
salmart-devforce-pushed the fix/convert-rotate-to-timedjob branch from 25fe82d to 96b0d16CompareJune 19, 2025 12:12

@AltahrimAltahrim left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You may need to adjust PR title :)

@salmart-devsalmart-dev changed the title fix: make Rotate a TimedJob and run every 5sfix: make Rotate a TimedJob and run every hourJun 19, 2025
@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport 30

@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport stable30

@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport to stable30

@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport to stable31

@salmart-dev
salmart-devforce-pushed the fix/convert-rotate-to-timedjob branch 2 times, most recently from 1235c6a to 542b124CompareJune 20, 2025 16:31
public function __construct(ITimeFactory $time) {
parent::__construct($time);

$this->setInterval(3600);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should eliminate this magic number (stick in in RotationTrait?) and then also use it elsewhere and keep rotation consistent across the board:

$this->setInterval(60 * 60 * 3);

$this->setInterval(60 * 60 * 3);

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.

Why not, but let’s not delay this PR any longer for this. It can be done later.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But what if a 1h junk is much bigger than the configured limit?
previously it ran on every execution?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@nickvergessen we discussed this here #53420 (comment) and concluded to set it to 1h, could you check with Côme or Benjamin?

@come-nc
come-ncforce-pushed the fix/convert-rotate-to-timedjob branch from 542b124 to 58aa06bCompareJune 23, 2025 10:22
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
@come-nc
come-ncforce-pushed the fix/convert-rotate-to-timedjob branch from 58aa06b to 5fef6f1CompareJune 24, 2025 07:55
@come-nc
come-nc merged commit 8ee06ed into masterJun 24, 2025
@come-nc
come-nc deleted the fix/convert-rotate-to-timedjob branch June 24, 2025 08:27
@github-actions

Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@salmart-dev@nickvergessen@icewind1991@joshtrichards@Altahrim@come-nc@skjnldsv