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

Additional info for rabbitmqctl rename_cluster_node man page #1641

Merged
merged 2 commits into from
Jul 19, 2018

Conversation

blgm
Copy link
Contributor

@blgm blgm commented Jul 12, 2018

Proposed Changes

The man page for rabbitmqctl rename_cluster_node currently explains that it only changes the local database. I have added information about the other steps required to successfully rename a node, as I was unable to find any.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

Thanks to @dumbbell for helping me figure out the instructions

The man page currently explains that it only changes the local database.
I have added information about the other steps required to successfully
rename a node.
@lukebakken
Copy link
Collaborator

Thanks! @dumbbell @michaelklishin this appears to be the only section of the web site that mentions this command (link). Should that link to the html version of this manual page?

Finally ... should rabbitmqctl be changed to rename these directories for the user?

.sp
.It
Rename the node in
.Ar /etc/rabbitmq/rabbitmq-env.conf
Copy link
Member

Choose a reason for hiding this comment

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

I would say

If you have /etc/rabbitmq/rabbitmq-env.conf and configured the node name there, update this configuration.

Because not many people are using rabbitmq-env.conf, thus they shouldn't be afraid if the file is missing.

and to configure the new node name.
For example:
.sp
.Bl -bullet -compact
Copy link
Member

Choose a reason for hiding this comment

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

Please use a numbered list here (.Bl -enum -compact).

.sp
.Dl mv /var/lib/rabbitmq/mnesia/rabbit\\@misshelpful /var/lib/rabbitmq/mnesia/rabbit\\@cordelia
.Dl mv /var/lib/rabbitmq/mnesia/rabbit\\@misshelpful-rename /var/lib/rabbitmq/mnesia/rabbit\\@cordelia-rename
.Dl mv /var/lib/rabbitmq/mnesia/rabbit\\@misshelpful-plugins-expand /var/lib/rabbitmq/mnesia/rabbit\\@cordelia-plugins-expand
Copy link
Member

Choose a reason for hiding this comment

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

I suggest you replace the three .Dl by this block:

.Bd -literal -offset indent -compact
mv \\
  /var/lib/rabbitmq/mnesia/rabbit@misshelpful \\
  /var/lib/rabbitmq/mnesia/rabbit@cordelia
mv \\
  /var/lib/rabbitmq/mnesia/rabbit@misshelpful-rename \\
  /var/lib/rabbitmq/mnesia/rabbit@cordelia-rename
mv \\
  /var/lib/rabbitmq/mnesia/rabbit@misshelpful-plugins-expand \\
  /var/lib/rabbitmq/mnesia/rabbit@cordelia-plugins-expand
.Ed

This way, the command lines are split to remain in the 80-column limit and are more readable.

@dumbbell
Copy link
Member

@lukebakken: Yes, perhaps rabbitmqctl(8) should do those steps. I don't know why it doesn't do that in the first place; there must be a reason for that. About the link to the manpage, I have no opinion on that :-)

No matter what we decide about an improvement to rabbitmqctl(8), I believe the doc improvement should be committed when ready.

@michaelklishin
Copy link
Member

It doesn't do that because in practice those who want to be more defensive against hostname changes override data directory path to not include hostnames to begin with, e.g. the tile has been doing that since 2014 or so, when this command was introduced to make migration to a different host/node naming scheme possible.

@michaelklishin
Copy link
Member

That said, it could detect that the current node data directory is one of the old names and move it since we know for sure that the node is stopped when this command runs.

@lukebakken
Copy link
Collaborator

Thanks for that additional info.

@blgm
Copy link
Contributor Author

blgm commented Jul 17, 2018

Hi @dumbbell. Thank you for your feedback. I have made the improvements you suggested. Please let me know if there's anything else you'd like to see improved.

@dumbbell dumbbell merged commit ac5db6f into rabbitmq:master Jul 19, 2018
dumbbell added a commit that referenced this pull request Jul 19, 2018
... to keep it under the 80-column limit.

References #1641.
dumbbell pushed a commit that referenced this pull request Jul 19, 2018
(cherry picked from commit 49ed422)
dumbbell added a commit that referenced this pull request Jul 19, 2018
... to keep it under the 80-column limit.

References #1641.

(cherry picked from commit 7c941af)
@dumbbell dumbbell added this to the 3.7.8 milestone Jul 19, 2018
@dumbbell dumbbell added the doc label Jul 19, 2018
@dumbbell dumbbell self-assigned this Jul 19, 2018
@dumbbell
Copy link
Member

Thank you @blgm for the patch!

@blgm blgm deleted the rename_node_docs_improvement branch July 20, 2018 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants