Skip to content

Introduce a new config parameter, total_memory_available_override_value, to override total amount of memory available to the node #1234

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

Merged
merged 2 commits into from
May 26, 2017

Conversation

dcorbacho
Copy link
Contributor

Closes #1224

… total memory

rabbitmq-server#1224
[#145450413]
@michaelklishin michaelklishin changed the title Use new config parameter total_memory_available_override_value to set total memory Introduce a new config parameter, total_memory_available_override_value, to override total amount of memory available to the node May 26, 2017
@@ -246,6 +246,10 @@
%%
%% {memory_monitor_interval, 2500},

%% The total memory available can be calculated from the OS resources
%% - default option - or provided as a configuration parameter:
%% {total_memory_available_override_value, "5000MB"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ nitpick ahead!

Since this value is parsed via rabbit_resource_monitor_misc:parse_information_unit, 5000MB will be interpreted as the powers-of-10 value, rather than powers-of-2. The user may not expect this behavior, I'm not sure.

Related to #1233 and #1235

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is why #1233 was reported

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see. What do you think about changing the comment text to 5000MiB or 5GiB so it's clear to the user that those abbreviations are supported?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We support MiB, GiB and such, so can change rabbitmq.config.example to use that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Units are specified just above:

%% k, kiB: kibibytes (2^10 bytes)
. The problem is that we report the wrong unit back to the user here: https://github.com/rabbitmq/rabbitmq-server/blob/master/src/vm_memory_monitor.erl#L240, using MB when the conversion applied for that log message is MiB.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dcorbacho thanks for pointing that out. I should have read the entire file rather than just this diff.

@thomas-riccardi
Copy link

Documentation is missing for total_memory_available_override_value at https://www.rabbitmq.com/configure.html (or anywhere else on the website it seems).

@michaelklishin
Copy link
Collaborator

michaelklishin commented Nov 20, 2017 via email

@thomas-riccardi
Copy link

Yes indeed, but the https://www.rabbitmq.com/configure.html page says its list is the complete list of configurable variables:

Variables Configurable in rabbitmq.config
Many users of RabbitMQ never need to change any of these values, and some are fairly obscure. However, for completeness they are all listed here.

It currently is not complete.

@michaelklishin
Copy link
Collaborator

@thomas-riccardi fair enough. See rabbitmq/rabbitmq-website@62b5267.

@thomas-riccardi
Copy link

Perfect, thanks !
I was not aware of this repository, I did search some source link from the footer and could not find any: it could be made more visible.

@michaelklishin
Copy link
Collaborator

michaelklishin commented Nov 21, 2017

@thomas-riccardi we are aware of that. Currently the plan is to make every guide link to its source on GitHub. Our small team is unlikely to get to it any time soon, though :(

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.

4 participants