accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Replace `admin signalShutdown` with a better name

Open dlmarion opened this issue 11 months ago • 6 comments

Command was added at:

https://github.com/apache/accumulo/pull/5193/files#diff-0632e2094904d132ddf08b3f00a0b682964e9045130748e7d9a31e93e630f6f1R325

It was suggested we might have a better name

dlmarion avatar Apr 09 '25 20:04 dlmarion

The genesis for this issue was a comment that maybe we shouldn't use camel case names for cli commands. Looking at Admin, seems like using camel case is the standard.

    cl.addCommand("serviceStatus", serviceStatusCommandOpts);
    cl.addCommand("changeSecret", changeSecretCommand);
    cl.addCommand("checkTablets", checkTabletsCommand);
    cl.addCommand("deleteZooInstance", deleteZooInstOpts);
    cl.addCommand("dumpConfig", dumpConfigCommand);
    cl.addCommand("fate", fateOpsCommand);
    cl.addCommand("signalShutdown", gracefulShutdownCommand);
    cl.addCommand("listInstances", listInstancesOpts);
    cl.addCommand("locks", tServerLocksOpts);
    cl.addCommand("ping", pingCommand);
    cl.addCommand("restoreZoo", restoreZooOpts);
    cl.addCommand("randomizeVolumes", randomizeVolumesOpts);
    cl.addCommand("stop", stopOpts);
    cl.addCommand("stopAll", stopAllOpts);
    cl.addCommand("stopManager", stopManagerOpts);
    cl.addCommand("stopMaster", stopMasterOpts);
    cl.addCommand("verifyTabletAssigns", verifyTabletAssignmentsOpts);
    cl.addCommand("volumes", volumesCommand);

dlmarion avatar Apr 10 '25 17:04 dlmarion

hey @dlmarion can u pls give a bit information about the issue?? :)

richochetclementine1315 avatar May 18 '25 19:05 richochetclementine1315

If we've had a release with the old names, we can keep them as a deprecated alias. Since these names are typed on the command-line, rather than representing code, I think more normal command naming convention for these would be something like "stop-manager" instead of "stopManager", so they can be more easily read and written by humans interacting with the command-line.

ctubbsii avatar May 19 '25 18:05 ctubbsii

For "signalShutdown", maybe "request-shutdown" or "send-shutdown-signal"?

ctubbsii avatar May 19 '25 18:05 ctubbsii

@richochetclementine1315 it looks like you are working on a fix for this. I'll assign you to this ticket for now.

DomGarguilo avatar May 21 '25 16:05 DomGarguilo

Moved to the 4.0 milestone based on conversation in #5565

dlmarion avatar May 28 '25 17:05 dlmarion

I am going to explore merging the admin stop and admin signalShutdown commands. https://github.com/apache/accumulo/pull/5734#discussion_r2214054262

keith-turner avatar Jul 17 '25 18:07 keith-turner

#5745 removed the command and merged it into the existing stop command

keith-turner avatar Jul 25 '25 20:07 keith-turner