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

Make it easier to know how to set the advertised address for a server process #4976

Open
keith-turner opened this issue Oct 11, 2024 · 4 comments
Labels
enhancement This issue describes a new feature, improvement, or optimization.
Milestone

Comments

@keith-turner
Copy link
Contributor

keith-turner commented Oct 11, 2024

Is your feature request related to a problem? Please describe.

In Accumulo 2.x the help for the tablet server would show the following.

$accumulo tserver --help
Usage: tserver [options]
  Options:
    -a, --address
      address to bind to
    -h, -?, --help, -help

    -p, -props, --props
      Sets path to accumulo.properties.The classpath will be searched if this 
      property is not set
    -o
      Overrides configuration set in accumulo.properties (but NOT system-wide 
      config set in Zookeeper). Expected format: -o <key>=<value>
      Default: []

Making it easy to know that you needed to run accumulo tsever -a aHostName to advertise the tserver process using aHostName.

In accumulo 4.0.0-SNAPSHOT running help shows the following.

$ ./accumulo tserver --help
Usage: tserver [options]
  Options:
    -h, -?, --help, -help

    -p, -props, --props
      Sets path to accumulo.properties.The classpath will be searched if this 
      property is not set
    -o
      Overrides configuration set in accumulo.properties (but NOT system-wide 
      config set in Zookeeper). Expected format: -o <key>=<value>
      Default: []

And to do the same thing as 2.x this command needs to be run accumulo tserver -o general.process.bind.addr=aHostName. The help does not help you figure this out. I looked at source code to figure this out and it took a while.

Describe the solution you'd like

It would be nice to make this common operation easier. Either add something to help message for servers or bring the -a option back.

@keith-turner keith-turner added the enhancement This issue describes a new feature, improvement, or optimization. label Oct 11, 2024
@keith-turner keith-turner added this to the 4.0.0 milestone Oct 11, 2024
@ctubbsii
Copy link
Member

This was done in #3192 (and in #3197) for 3.0.0, and was documented in the published release notes at https://accumulo.apache.org/release/accumulo-3.0.0/

It was done largely in response to the discussion at #3189 (comment) as a follow-on to a decision to avoid adding a bunch of options to the ServerOpts in that issue, and instead favoring our published documentation at https://accumulo.apache.org/docs/2.x/configuration/server-properties3

So, this is already documented in the published docs on the website for 3.0, the convenience scripts for launching have been updated to use it automatically, and it was documented in the release notes for 3.0. I'm not sure how often people do bin/accumulo tserver -?, but I'd be surprised if users are normally calling bin/accumulo tserver directly, instead of through a provided convenience cluster management script, systemd unit, or similar entry point.

I think the reasoning for removing it was sound, so I would not want to see it come back. But, I would be in favor of adding a note in the output of bin/accumulo tserver -? to document that it will change in a future release, and making it more prominent in the 3.0 release notes.

Another option is to add a warning when it is used in 2.1. That might be a bit noisy, though, unless you also backport the new property, so people can migrate away from using -a to avoid the warning. I'm not sure how difficult it will be to backport... probably not very difficult, but I'm not sure.

@ctubbsii ctubbsii modified the milestones: 4.0.0, 3.1.0, 2.1.4 Oct 12, 2024
@ctubbsii
Copy link
Member

If this is addressed, it should probably be addressed in 2.1, since that's where we'd want to add the extra documentation, or backport the configuration to.

@keith-turner
Copy link
Contributor Author

If this is addressed, it should probably be addressed in 2.1, since that's where we'd want to add the extra documentation, or backport the configuration to.

if the -o general.process.bind.addr=aHostName is supported in 2.1, then documenting it in the help message there and merging that forward makes sense to me.

@ctubbsii
Copy link
Member

Maybe we could shorten it to rpc.bind.address and backport that to 2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue describes a new feature, improvement, or optimization.
Projects
None yet
Development

No branches or pull requests

2 participants