Skip to content

feat(payload-cloud): set up cron jobs on init#10106

Merged
denolfe merged 18 commits into
mainfrom
cloud-jobs-single-instance
Jan 9, 2025
Merged

feat(payload-cloud): set up cron jobs on init#10106
denolfe merged 18 commits into
mainfrom
cloud-jobs-single-instance

Conversation

@GermanJablo
Copy link
Copy Markdown
Contributor

If the user has tasks configured, we set up cron jobs on init.
We also make sure to only run on one instance using a instance identifier stored in a global.

This adds a new property to the payloadCloudPlugin: jobs.

@socket-security
Copy link
Copy Markdown

socket-security Bot commented Dec 20, 2024

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

@GermanJablo GermanJablo marked this pull request as draft December 20, 2024 18:53
Comment thread packages/payload-cloud/src/plugin.ts Outdated
Comment thread packages/payload-cloud/src/plugin.ts Outdated
Comment thread packages/payload-cloud/src/plugin.ts Outdated
@GermanJablo GermanJablo marked this pull request as ready for review December 20, 2024 20:00
Comment thread packages/payload-cloud/src/types.ts Outdated
Comment thread packages/payload-cloud/src/plugin.ts
Comment thread packages/payload-cloud/src/types.ts Outdated
Copy link
Copy Markdown
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

What are your thoughts?


function waitRandomTime(): Promise<void> {
const min = 1000 // 1 second in milliseconds
const max = 5000 // 5 seconds in milliseconds
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.

The window of time does not seem long enough to gaurentee that multiple instances aren't self identifying as the cron runner.
A min of 1 second between starting up the app instance, connecting to the database, and updating the global all adds up to a much larger tolerance needed if I had to guess.

I'm not sure how much you'ved tested this, but to me we should remove the waitRandomTime() entirely and just let the instances write their unique id to the global as they all come up. Then in a setTimeout with 2 full minutes do lines 140 - 155.

Comment thread docs/jobs-queue/queues.mdx
Comment on lines +59 to +63
/**
* Queue cron jobs automatically on payload initialization.
* @remark this property should not be used on serverless platforms like Vercel
*/
autoRun?: ((payload: Payload) => CronConfig[] | Promise<CronConfig[]>) | CronConfig[]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same as the comment above. This is from the other PR... ¿¿???

Comment on lines +103 to +108
const DEFAULT_CRON = '* * * * *'
const DEFAULT_LIMIT = 10
const DEFAULT_CRON_JOB = {
cron: DEFAULT_CRON,
limit: DEFAULT_LIMIT,
queue: 'default (every minute)',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now that cronjobs are defined in config.jobs.autoRun and not in the plugin, where this "default cron job" should be documented. I copy the internal question:

Should the default cronjob:

  1. be set always after jobs.autoRun if it returns an empty array []?
  2. be set only in Payload Cloud Plugin if jobs autoRun returns an empty array?
  3. there should not be a default cronjob.

Personally I like option 3 better.
If we go with option 2 where would it be documented in tsdoc? the payload cloud plugin will not have any option to configure cron jobs, just to enable or disable it

@denolfe denolfe merged commit 36e50dd into main Jan 9, 2025
@denolfe denolfe deleted the cloud-jobs-single-instance branch January 9, 2025 04:30
@github-actions
Copy link
Copy Markdown
Contributor

🚀 This is included in version v3.16.0

kendelljoseph pushed a commit that referenced this pull request Feb 21, 2025
If the user has tasks configured, we set up cron jobs on init.
We also make sure to only run on one instance using a instance
identifier stored in a global.

This adds a new property to the payloadCloudPlugin: `jobs`.
Sign up for free to 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.

4 participants