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

Enhancement: Add New Helm Chart Parameter for Broker Message Spool Limit #99

Closed
itsJamilAhmed opened this issue Jan 19, 2021 · 7 comments · Fixed by #114
Closed

Enhancement: Add New Helm Chart Parameter for Broker Message Spool Limit #99

itsJamilAhmed opened this issue Jan 19, 2021 · 7 comments · Fixed by #114

Comments

@itsJamilAhmed
Copy link

What
Add a new helm chart parameter to configure the max spool usage limit on deployment of the brokers

Why
Presently, a chart parameter of storage.size is available to control the size of the persistent volume claim for each broker pod.
It can be assumed that a user will specify a value to override the default to support larger message spools with the larger volume.

There is not however any means to control the broker's configured max spool usage limit as part of deploying the stateful set. Currently this is defaulting to 1500MB and needs config to be subsequently issued following broker creation to increase it.

How
There is already a container startup config key available to manage this, requesting the addition of a new helm chart parameter solace.maxspoolusage that allows a value other than the 1500MB default to be collected.

The collected value can then be exported as an environment variable in the file solaceConfigMap.yaml alongside the existing config keys such as export system_scaling_maxconnectioncount.

Impact Assessment
A new optional parameter that only when present can export the appropriate config key. If not present then nothing is exported and the broker level defaults, or user issued config change, continue to apply for existing chart deployments that may execute an upgrade.

@itsJamilAhmed itsJamilAhmed changed the title Enhancement: Add New Helm Chart Parameter for Broker Message Spool Enhancement: Add New Helm Chart Parameter for Broker Message Spool Limit Jan 19, 2021
@dwongkm
Copy link

dwongkm commented Jan 19, 2021

thanks, Jamil. I have created an entry in JIRA to keep track of this request.

Douglas

@bczoma
Copy link
Collaborator

bczoma commented Jan 27, 2021

In the short term export system_scaling_maxqueuemessagecount can be manually added to https://github.com/SolaceProducts/pubsubplus-kubernetes-quickstart/blob/master/pubsubplus/templates/solaceConfigMap.yaml#L19 but care must be taken to also update https://github.com/SolaceProducts/pubsubplus-kubernetes-quickstart/blob/master/pubsubplus/templates/solaceStatefulSet.yaml#L46-L84 if required, consulting the Solace documentation for Additional System Resources.
A longer term solution will be devised to make this automated.

@itsJamilAhmed
Copy link
Author

With the config key called messagespool/maxspoolusage being the one of interest here to configure the spool size on broker startup, is this change not exclusive to just the file solaceConfigMap.yaml? That's where all the other config keys get exported.

Am I missing something or should this change not be along the lines of adding:

{{- if eq .Values.solace.maxspoolusage }}
    export messagespool_maxspoolusage="{{.Values.solace.maxspoolusage}}"

after line 18 of solaceConfigMap.yaml

The key you reference in your comment (system_scaling_maxconnectioncount) is already handled no?

@bczoma
Copy link
Collaborator

bczoma commented Jan 27, 2021

@itsJamilAhmed You're right, system_scaling_maxqueuemessagecount is the one I thought and updated my original comment with.
But it seems you are only interested in messagespool/maxspoolusage ?

@itsJamilAhmed
Copy link
Author

Yep, just a parameter to modify the message spool limit in MB is all I'm after with this particular request.

With the system/scaling/maxqueuemessagecount defaulting at 100 million messages, it's more than enough that I'm fine without something for that. 😊

@bczoma
Copy link
Collaborator

bczoma commented Jan 28, 2021

Sounds good, will add it to the next rev

@bczoma
Copy link
Collaborator

bczoma commented Feb 10, 2022

https://github.com/SolaceProducts/pubsubplus-kubernetes-quickstart/pull/109 has added support to provide any additional env variable to the PubSub+ container, which can also be used for config key.
Future update will add specific support to all supported scaling parameters.

PhilippeKhalife pushed a commit that referenced this issue May 10, 2022
* Documented support and use of Ingress
* Support to individually specify all PubSub+ scaling parameters - also fixes Enhancement: Add New Helm Chart Parameter for Broker Message Spool Limit #99
* Support to use single mount storage-group for the broker
* Allow smaller Monitor pod CPU, Memory and storage requirements in an HA deployment
* Fixed Helm error with custom service annotation - fixes issue [BUG] Helm error with custom service annotation #112
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