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

Don't use RABBITMQ_SERVER_ERL_ARGS #841

Merged
merged 1 commit into from
Jun 23, 2020

Conversation

jeckersb
Copy link
Contributor

Instead, use RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS

Quoted from https://www.rabbitmq.com/configure.html#supported-environment-variables:

"
RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value.
"

The default value of RABBITMQ_SERVER_ERL_ARGS on *nix is:

+P 1048576 +t 5000000 +stbt db +zdbbl 128000

So currently, if ipv6/tls is enabled, and the caller does not provide
its own default value of RABBITMQ_SERVER_ERL_ARGS, it will end up
overwriting those server defaults.

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Instead, use RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS

Quoted from https://www.rabbitmq.com/configure.html#supported-environment-variables:

"
RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value.
"

The default value of RABBITMQ_SERVER_ERL_ARGS on *nix is:

+P 1048576 +t 5000000 +stbt db +zdbbl 128000

So currently, if ipv6/tls is enabled, and the caller does not provide
its own default value of RABBITMQ_SERVER_ERL_ARGS, it will end up
overwriting those server defaults.
@jeckersb
Copy link
Contributor Author

@mbaldessari

@wyardley
Copy link
Contributor

@jeckersb If this is a result of changes, do you know which version the change happened? Does this PR risk breaking things on older RabbitMQ versions (and if so, which ones)?

@wyardley
Copy link
Contributor

ah, I see, appending vs. overriding (https://www.rabbitmq.com/runtime.html)
Assuming this works on all older versions, seems good to me.

@wyardley wyardley requested a review from ekohl June 23, 2020 19:08
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I don't know too much about RabbitMQ but does this remove the option to use RABBITMQ_SERVER_ERL_ARGS?

@jeckersb
Copy link
Contributor Author

I don't know too much about RabbitMQ but does this remove the option to use RABBITMQ_SERVER_ERL_ARGS?

No, if you want to set RABBITMQ_SERVER_ERL_ARGS you can still pass it in as part of the environment_variables hash. This just changes the behavior when the ipv6 or ssl_erl_dist flags are enabled, since they require munging the server cmdline to enable the right flags. With this patch the munging is done against the RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS variable so the rabbitmq system defaults are preserved.

@ekohl ekohl merged commit 71a0631 into voxpupuli:master Jun 23, 2020
@ekohl ekohl added the enhancement New feature or request label Jun 23, 2020
@jeckersb
Copy link
Contributor Author

@jeckersb If this is a result of changes, do you know which version the change happened? Does this PR risk breaking things on older RabbitMQ versions (and if so, which ones)?

The ADDITIONAL variable was added here - rabbitmq/rabbitmq-server@9447629

Which is part of RabbitMQ v3.4.0, and per https://www.rabbitmq.com/versions.html the 3.3 series went out of support on 31 March 2015. So in theory it could break someone who insists on using a RabbitMQ version that has been out of support for over 5 years now, but really insists on the latest puppet-rabbitmq :)

openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request Jul 7, 2020
* Update tripleo-heat-templates from branch 'master'
  - Merge "Don't set RABBITMQ_SERVER_ERL_ARGS"
  - Don't set RABBITMQ_SERVER_ERL_ARGS
    
    From the RabbitMQ docs[1]:
    
    "RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
    used when invoking the RabbitMQ Server. This should be overridden for
    debugging purposes only. Overriding this variable replaces the default
    value."
    
    We do not need any of the current values because of this commit from
    2015 in rabbitmq 3.5.4:
    
    rabbitmq/rabbitmq-server@5c3c0e6
    
    In which the "+K true" and "-kernel inet_default_connect_options
    "[{nodelay,true}]" options were hard-coded to the rabbitmq-server
    script.  Meanwhile the "+P 1048576" configuration was moved to the
    rabbitmq-env script under the SERVER_ERL_ARGS variable, which is the
    defaults used when RABBITMQ_SERVER_ERL_ARGS is unset.
    
    [1] https://www.rabbitmq.com/configure.html#supported-environment-variables
    
    Let's also move the current RabbitAdditionalErlArgs to a new dedicated
    override hiera key.
    
    NB: To get this properly working we need a puppet-rabbitmq that contains
        voxpupuli/puppet-rabbitmq#841
    Depends-On: I3bf244a70538209773804eb85fae6be035c587f4
    
    Closes-Bug: #1884922
    
    Change-Id: I567839785a72813a382a00253562894e19eb6715
openstack-mirroring pushed a commit to openstack-archive/tripleo-heat-templates that referenced this pull request Jul 7, 2020
From the RabbitMQ docs[1]:

"RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value."

We do not need any of the current values because of this commit from
2015 in rabbitmq 3.5.4:

rabbitmq/rabbitmq-server@5c3c0e6

In which the "+K true" and "-kernel inet_default_connect_options
"[{nodelay,true}]" options were hard-coded to the rabbitmq-server
script.  Meanwhile the "+P 1048576" configuration was moved to the
rabbitmq-env script under the SERVER_ERL_ARGS variable, which is the
defaults used when RABBITMQ_SERVER_ERL_ARGS is unset.

[1] https://www.rabbitmq.com/configure.html#supported-environment-variables

Let's also move the current RabbitAdditionalErlArgs to a new dedicated
override hiera key.

NB: To get this properly working we need a puppet-rabbitmq that contains
    voxpupuli/puppet-rabbitmq#841
Depends-On: I3bf244a70538209773804eb85fae6be035c587f4

Closes-Bug: #1884922

Change-Id: I567839785a72813a382a00253562894e19eb6715
openstack-mirroring pushed a commit to openstack-archive/tripleo-heat-templates that referenced this pull request Jul 11, 2020
From the RabbitMQ docs[1]:

"RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value."

We do not need any of the current values because of this commit from
2015 in rabbitmq 3.5.4:

rabbitmq/rabbitmq-server@5c3c0e6

In which the "+K true" and "-kernel inet_default_connect_options
"[{nodelay,true}]" options were hard-coded to the rabbitmq-server
script.  Meanwhile the "+P 1048576" configuration was moved to the
rabbitmq-env script under the SERVER_ERL_ARGS variable, which is the
defaults used when RABBITMQ_SERVER_ERL_ARGS is unset.

[1] https://www.rabbitmq.com/configure.html#supported-environment-variables

Let's also move the current RabbitAdditionalErlArgs to a new dedicated
override hiera key.

NB: To get this properly working we need a puppet-rabbitmq that contains
    voxpupuli/puppet-rabbitmq#841
Depends-On: I3bf244a70538209773804eb85fae6be035c587f4

Closes-Bug: #1884922

Change-Id: I567839785a72813a382a00253562894e19eb6715
(cherry picked from commit 938166b)
openstack-mirroring pushed a commit to openstack-archive/tripleo-heat-templates that referenced this pull request Jul 13, 2020
From the RabbitMQ docs[1]:

"RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value."

We do not need any of the current values because of this commit from
2015 in rabbitmq 3.5.4:

rabbitmq/rabbitmq-server@5c3c0e6

In which the "+K true" and "-kernel inet_default_connect_options
"[{nodelay,true}]" options were hard-coded to the rabbitmq-server
script.  Meanwhile the "+P 1048576" configuration was moved to the
rabbitmq-env script under the SERVER_ERL_ARGS variable, which is the
defaults used when RABBITMQ_SERVER_ERL_ARGS is unset.

[1] https://www.rabbitmq.com/configure.html#supported-environment-variables

Let's also move the current RabbitAdditionalErlArgs to a new dedicated
override hiera key.

NB: To get this properly working we need a puppet-rabbitmq that contains
    voxpupuli/puppet-rabbitmq#841
Depends-On: I3bf244a70538209773804eb85fae6be035c587f4

Closes-Bug: #1884922

Change-Id: I567839785a72813a382a00253562894e19eb6715
(cherry picked from commit 938166b)
openstack-mirroring pushed a commit to openstack-archive/tripleo-heat-templates that referenced this pull request Jul 22, 2020
From the RabbitMQ docs[1]:

"RABBITMQ_SERVER_ERL_ARGS - Standard parameters for the erl command
used when invoking the RabbitMQ Server. This should be overridden for
debugging purposes only. Overriding this variable replaces the default
value."

We do not need any of the current values because of this commit from
2015 in rabbitmq 3.5.4:

rabbitmq/rabbitmq-server@5c3c0e6

In which the "+K true" and "-kernel inet_default_connect_options
"[{nodelay,true}]" options were hard-coded to the rabbitmq-server
script.  Meanwhile the "+P 1048576" configuration was moved to the
rabbitmq-env script under the SERVER_ERL_ARGS variable, which is the
defaults used when RABBITMQ_SERVER_ERL_ARGS is unset.

[1] https://www.rabbitmq.com/configure.html#supported-environment-variables

Let's also move the current RabbitAdditionalErlArgs to a new dedicated
override hiera key.

NB: To get this properly working we need a puppet-rabbitmq that contains
    voxpupuli/puppet-rabbitmq#841
Depends-On: I3bf244a70538209773804eb85fae6be035c587f4

Closes-Bug: #1884922

Change-Id: I567839785a72813a382a00253562894e19eb6715
(cherry picked from commit 938166b)
apevec pushed a commit to redhat-openstack/rdoinfo that referenced this pull request Mar 22, 2021
We want this so we include a number of fixes.
In particular we want:
- voxpupuli/puppet-rabbitmq#841 as it
  improves some default settings around performance

- 877f32042a5a to remove the harmless but worrying facter error

In order to do so we also need to update puppet-systemd to
a version that includes e556d27868b250509b08511e80374a002c9c1649
"Add selinux_ignore_defaults support to dropin_file and service_limits"
because puppet-rabbitmq v10.1.2 uses that. We move to 2.10.0 as that
is the first released version that includes it.
The puppet-systemd rebase is only needed on train and ussuri as victoria
already pins to a version that includes e556d27868.

Tested by deploying a largeish OSP 16.2 Train env with puppet-systemd at
2.10.0 and puppet-rabbitmq at v10.1.2

Change-Id: Ibf511265b7f56f96f5b84538d28cf40034f650fb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants