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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb7e6af
Use new config parameter total_memory_available_override_value to set…
dcorbacho May 26, 2017
1b97635
Missing comma in config example
michaelklishin May 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/rabbitmq.config.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

rabbitmq-server/docs/rabbitmq.config.example

Line 230 in 1b97635

%% 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.


%% Set disk free limit (in bytes). Once free disk space reaches this
%% lower bound, a disk alarm will be set - see the documentation
%% listed above for more details.
Expand Down
30 changes: 23 additions & 7 deletions src/vm_memory_monitor.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,20 @@
%%----------------------------------------------------------------------------

get_total_memory() ->
try
get_total_memory(os:type())
catch _:Error ->
rabbit_log:warning(
"Failed to get total system memory: ~n~p~n~p~n",
[Error, erlang:get_stacktrace()]),
unknown
case application:get_env(rabbit, total_memory_available_override_value) of
{ok, Value} ->
case rabbit_resource_monitor_misc:parse_information_unit(Value) of
{ok, ParsedTotal} ->
ParsedTotal;
{error, parse_error} ->
rabbit_log:warning(
"The override value for the total memmory available is "
"not a valid value: ~p, getting total from the system.~n",
[Value]),
get_total_memory_from_os()
end;
undefined ->
get_total_memory_from_os()
end.

get_vm_limit() -> get_vm_limit(os:type()).
Expand Down Expand Up @@ -164,6 +171,15 @@ code_change(_OldVsn, State, _Extra) ->
%%----------------------------------------------------------------------------
%% Server Internals
%%----------------------------------------------------------------------------
get_total_memory_from_os() ->
try
get_total_memory(os:type())
catch _:Error ->
rabbit_log:warning(
"Failed to get total system memory: ~n~p~n~p~n",
[Error, erlang:get_stacktrace()]),
unknown
end.

set_mem_limits(State, MemLimit) ->
case erlang:system_info(wordsize) of
Expand Down