Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: Specify resource requests and limits for trace job #50

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

dalehamel
Copy link
Member

Fixes #49

This sets the trace job to be burstable, with a very low CPU requirement (1/10th of a CPU), to ensure that the job can be easily scheduled without being disruptive. It sets a maximum of 1 full CPU as well.

Similarly, for memory it requires 100mb, with a maximum of 1Gb.

We can tune these up or down as need be, but for now this should be a wide range to operate in.

If #47 is merged, then requests/limits should be applied to the initContainer as well.

@fntlnz what do you think?

@dalehamel dalehamel force-pushed the resource-requests-limits branch from 1c11a16 to b5bb5df Compare January 27, 2019 22:44
@dalehamel dalehamel changed the title Specify resource requests and limits for trace job fix: Specify resource requests and limits for trace job Jan 27, 2019
@fntlnz
Copy link
Member

fntlnz commented Jan 28, 2019

@dalehamel I was wondering if this could have any impact in programs using maps like for sum and hist but you set the hard limits to 1 CPU and 1G of Memory so it seams reasonable.

@dalehamel
Copy link
Member Author

maps like for sum and hist but you set the hard limits to 1 CPU and 1G of Memory so it seams reasonable.

Yeah i think if this becomes an issue, someone will open a bug report and they'd be able to indicate what caused the resource usage to go above that.

1GB and 1 full CPU should be a lot

Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

This looks perfectly fine (and needed) to me, too. LGTM

@leodido leodido merged commit 60d7811 into iovisor:master Jan 28, 2019
@fntlnz
Copy link
Member

fntlnz commented Jan 28, 2019

Thanks @dalehamel

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

Successfully merging this pull request may close these issues.

3 participants