Skip to content

Fail accumulo-service stop/kill when no service name is given - #6537

Open
DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:emptyServiceName
Open

DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:emptyServiceName

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

Running accumulo-service manager stop without --all or an instance name silently does nothing. The script:

  • prints Stopping service process: with a blank name,
  • looks for the pid file accumulo-.pid, which doesn't exist,
  • exits 0.

The manager keeps running even though the command reported success. A later start then says manager_default_1 already running. kill has the same problem, with no output at all.

This PR adds a check in function main() before stop/kill are dispatched. When all of these are true:

  1. no service name was given
  2. --all was not passed
  3. the script is not in cluster mode (ACCUMULO_CLUSTER_ARG unset, so calls from accumulo-cluster are unaffected)

it exits 1 with this message, followed by the usage text:

Invalid arguments: 'stop' requires --all or a service (e.g. manager_default_1; see 'manager list')

Nothing is stopped, and the user is told what to pass instead.

Also:

  • The usage line changes from stop [--all | []] to stop [--all | ] (same for kill), so the name no longer looks optional.
  • invalid_args now prints its "Invalid arguments" line to stderr, matching the usage text it already sent there.

@DomGarguilo DomGarguilo added this to the 4.0.0 milestone Sep 15, 2026
@DomGarguilo DomGarguilo self-assigned this Sep 15, 2026
if [[ -z $command_name ]]; then
invalid_args "<command> cannot be empty"
fi
if [[ $command_name =~ ^(stop|kill)$ && $all_flag != 'true' && -z $ACCUMULO_CLUSTER_ARG && -z $service_name ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to add this arg check in the logic for the stop_service and kill_service functions?

@ddanielr ddanielr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The behavior change is beneficial. I'd switch this PR to the 2.1 branch though.
There was a good bit of work done to make sure the control scripts matched between 2.1 and main.

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.

2 participants