Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cron: Adapt to 5 minutes recommendation #19506

Merged
merged 1 commit into from
Feb 19, 2020
Merged

cron: Adapt to 5 minutes recommendation #19506

merged 1 commit into from
Feb 19, 2020

Conversation

michaelkuhn
Copy link
Contributor

cron is still assuming that it will be scheduled every 15 minutes. Adapt this to the new 5 minutes recommendation to avoid overlapping executions.

@gary-kim gary-kim added bug 3. to review Waiting for reviews labels Feb 17, 2020
@gary-kim gary-kim added this to the Nextcloud 19 milestone Feb 17, 2020
juliusknorr
juliusknorr approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@nickvergessen
Copy link
Member

That is actually intended. And back then I assumed it's 5min and running 14 means we can have up to 3 parallel crons.
So I would leave it like that

@ChristophWurst
Copy link
Member

Okay, but then the comment is a bit misleading, no? Should we adjust that with this info?

@nickvergessen
Copy link
Member

Yeah

ChristophWurst
ChristophWurst requested changes Feb 17, 2020
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Please adjust the comments to #19506 (comment). The value is actually corret.

@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 17, 2020
@michaelkuhn
cron: Adapt to 5 minutes recommendation
ab6a18f 
Update the comment to make clear that we ask for 14 minutes worth of
jobs on purpose so at most three parallel cron jobs are running.

Signed-off-by: Michael Kuhn <michael@ikkoku.de>
@michaelkuhn
Copy link
Contributor Author

Please adjust the comments to #19506 (comment). The value is actually corret.

Done, hope the new version is okay. :-)

@blizzz blizzz merged commit 29f610e into nextcloud:master Feb 19, 2020
@welcome
Copy link

welcome bot commented Feb 19, 2020

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

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

@ChristophWurst ChristophWurst ChristophWurst requested changes

@juliusknorr juliusknorr juliusknorr approved these changes

@nickvergessen nickvergessen nickvergessen approved these changes

@blizzz blizzz Awaiting requested review from blizzz

Assignees
No one assigned
Labels
2. developing Work in progress bug
Projects
None yet
Milestone
Nextcloud 19
Development

Successfully merging this pull request may close these issues.
6 participants
@michaelkuhn @nickvergessen @ChristophWurst @juliusknorr @blizzz @gary-kim