Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/rabbitmq] feature request: automatic configuration for schedulers threads using resources requests #13050

Closed
thomas-riccardi opened this issue Apr 15, 2019 · 9 comments
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@thomas-riccardi
Copy link
Contributor

Is this a request for help?:


Is this a BUG REPORT or FEATURE REQUEST? (choose one): feature request

Which chart:
stable/rabbitmq

What happened:
Since #13044 the schedulers threads are configurable from the chart values, with hardcoded default values.
The default value could be derived from the actual resources.requests.cpu (cf #3855 (comment)).

Prior to #13044 the default values were computed by rabbitmq: derived from the actual core count; thus #13044 could introduce performance regression with default values.

@juan131
Copy link
Collaborator

juan131 commented Apr 16, 2019

Hi @thomas-riccardi

As it was mentioned at #3855 there are two ways to automatically configure the scheduler threads:

  • Helm templating
    • It's not trivial at all since it'll require to perform some parameters transformations to do the calculations. E.g. resource.requests.cpu can be 100m, so it's necessary to transform that into a float.
  • Using the Downward API

I'd rather use the second approach for simplicity. What do you think @thomas-riccardi @tompizmor ?

@thomas-riccardi
Copy link
Contributor Author

thomas-riccardi commented Apr 16, 2019

Hi @juan141

I'm not sure either.

pros:

  • more flexibility in bash

cons:

  • less code in the already large shell command may be better
  • for resource.requests.cpu the only non number case is the milli m suffix: it could probably be handled in helm template (a possible rule is resource.requests.cpu <= 1 => 1 core)

However, a followup issue would raise the same questions for resource.limits.memory (to properly auto-configure the memory alarm), and there the variety of units is much bigger.

=> maybe it's better to do it in bash (and maybe it should be moved to the bitnami image? (Since #4591 the bitnami init script is half bypassed, leading to all kinds of problems, but this is another issue...))

@juan131
Copy link
Collaborator

juan131 commented Apr 17, 2019

I agree with you. Handling it in the Bitnami init scripts will provide us much more flexibility to implement the corresponding Bash logic without making the shell command any longer. However, there will be users complaining since they use the RabbitMQ chart with non-bitnami RabbitMQ docker images.

I think that once we (Bitnami) move RabbitMQ to a Bash approach (as we did on Redis or MariaDB). The Bitnami init scripts will be much easier to understand and users'll feel more comfortable with moving all this logic to the container itself (avoiding bypassing the init logic).

@stale
Copy link

stale bot commented May 17, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2019
@thomas-riccardi
Copy link
Contributor Author

bump to remove the lifecycle/stale label.

@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2019
@stale
Copy link

stale bot commented Jun 16, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 16, 2019
@juan131
Copy link
Collaborator

juan131 commented Jun 17, 2019

Hi @thomas-riccardi

Good news! We already moved the RabbitMQ so it's based on Bash scripts. See:

https://github.com/bitnami/bitnami-docker-rabbitmq/tree/master/3.7/debian-9/rootfs

Now we need to decide wether we want to replace the current command and we use the logic available in the Bash container or not. As I mentioned on a previous comment, there'll be user that'll complain for this change since they're not using the Bitnami image.

As an alternative, we could create a parameter in the template to decide whether to use a custom command or not (relying on the container init scripts).

@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 17, 2019
@stale
Copy link

stale bot commented Jul 17, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 17, 2019
@stale
Copy link

stale bot commented Jul 31, 2019

This issue is being automatically closed due to inactivity.

@stale stale bot closed this as completed Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants