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

Microservice jsonnet library ignores resource requests and limits #701

Closed
PatTheSilent opened this issue May 14, 2021 · 4 comments · Fixed by #793
Closed

Microservice jsonnet library ignores resource requests and limits #701

PatTheSilent opened this issue May 14, 2021 · 4 comments · Fixed by #793
Assignees

Comments

@PatTheSilent
Copy link

Describe the bug
Because of import ordering in tempo.libsonnet resouce requirements and limits in config.libsonnet are ignored.

config.libsonnet defines resource limits and requirements operations/jsonnet/microservices/config.libsonnet#L59

tempo.libsonnet imports config.libsonnet before all other relevant modules operations/jsonnet/microservices/tempo.libsonnet#L4

tempo_compactor_container from config.libsonnet is overwritten by tempo_compactor_container from compactor.libsonnet

It's not a big deal and one can easily set those in ones tanka environment but it's confusing because when looking only at config.libsonnet I'd expect those resource constraints to be present in PodSpecs.

I'd suggest changing config.libsonnet to

compactor: {
  replicas: 1,
  resources: {
    limits: {
      cpu: '',
      memory: ''
    },
    requests: {
      cpu: '',
      memory: ''
    }
  }
},

And then handle that in compactor.libsonnet

+ util.resourcesRequests($._config.compactor.resources.requests.cpu, $._config.compactor.resources.requests.mem)

I'm using compactor as an example because it's spiky resource consuption caused some throttling on my VMs without those requests and limits but this is true for all Tempo components.

To Reproduce
Use https://github.com/grafana/tempo/blob/main/operations/jsonnet/microservices/tempo.libsonnet

Expected behavior
Microservices have resource requirements and limits set

Environment:

  • Infrastructure: Kubernetes
  • Deployment tool: jsonnet

Additional Context
None

@joe-elliott
Copy link
Member

I think this would be a nice addition.

Currently we set limits directly on the container:

  tempo_querier_container+::
    k.util.resourcesRequests('500m', '2Gi') +
    k.util.resourcesLimits('3', '4Gi')

but I agree adding some structure around them could be good. If you are able to submit a PR please do!

@PatTheSilent
Copy link
Author

Haven't yet had the time to look at that. I'm hoping to find some time next week.

@kvrhdn
Copy link
Member

kvrhdn commented Jun 24, 2021

I can look into this, I was working on the tanka config anyway 🙂

@kvrhdn kvrhdn self-assigned this Jun 24, 2021
@PatTheSilent
Copy link
Author

@kvrhdn thanks! :)

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 a pull request may close this issue.

3 participants