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.

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

Add ability to set CPU and Memory for scheduled task #621

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidsiaw
Copy link
Contributor

Previously it was not possible to set a scheduled task's Memory or CPU usage, and it was pinned at 512MB of RAM and 128 units of CPU. This needs to change as some tasks require more memory.

@degicat degicat requested a review from Iorin0225 October 7, 2020 04:10
@degicat
Copy link

degicat commented Oct 7, 2020

@Iorin0225 please review this

@davidsiaw
Copy link
Contributor Author

@Iorin0225 sorry I meant to put @k2nr on this review but I think I messed up the command. I will remove you as reviewer for this PR

@davidsiaw davidsiaw requested review from k2nr and removed request for Iorin0225 October 7, 2020 04:11
@k2nr
Copy link
Member

k2nr commented Oct 12, 2020

I think it's better to set cpu and memory per task like this. Also, same as services, can we change cpu default to nil?

scheduled_tasks:
  - schedule: rate(5 minutes)
    command: some_task
    memory: 1024
  - ...

@davidsiaw
Copy link
Contributor Author

I think it's better to set cpu and memory per task like this

This is a little more complicated to do because we store this information per heritage. This will involve changing the model and moving a bunch of things around. For now the idea is just to make it modifiable.

can we change cpu default to nil?

We can, I just kept the CPU the same so we don't change too many things at once. I can put up a PR that sets the scheduled task CPU to default to nil. I can't think of any possible problems with that, but there could be problems that would emerge that I don't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants