Skip to content

Conversation

@dlmarion
Copy link
Contributor

@dlmarion dlmarion commented Feb 6, 2026

Closes #6086

@dlmarion dlmarion added this to the 4.0.0 milestone Feb 6, 2026
@dlmarion dlmarion self-assigned this Feb 6, 2026
@dlmarion
Copy link
Contributor Author

dlmarion commented Feb 6, 2026

There are two tests in CheckServerIT that are not passing

@dlmarion dlmarion marked this pull request as ready for review February 9, 2026 16:25
@keith-turner
Copy link
Contributor

Ran this locally to see what it looked like. This is great, could really improve and organize what is currently there. Was wondering about the following groupings of commands, not suggesting that for this PR just posting thoughts about eventual direction. Not sure if this would be good to do in this PR or not.

file group # wal,rfile
  create empty rfile
  rfile info
  gen splits
  split large
  wal info

volume group #
  list-volumes
  add volume ?
  
start group # start server processes
  start compactor
  start manager
   etc

server group  # management of running servers
 ping
 service status
 stop-all
 stop-managers
 stop-servers
 list-compactors
 zoo-zap

fate group # management of fate operations
 list
 fail
 kill

compaction group #these may be duplicated in the shell, not sure
 cancel 
 list

admin group  # whatever is not in another group above (dropping the current other group and letting admin be the catchall)

Also, there is probably a lot of room for improvements in follow on issues to get more consistency across commands.

Also wondering about the core, if there is a way to have those commands at the top level

@dlmarion
Copy link
Contributor Author

dlmarion commented Feb 9, 2026

@keith-turner - no issues here with renaming and regrouping things, but I'd rather do it in follow-on PRs, just so that this doesn't continue to grow.

Copy link
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

I'm not sure "admin" is the best catch-all because users can just throw new commands on the classpath. But, overall, I think this is headed in a good direction.

* under the License.
*/
package org.apache.accumulo.server.util;
package org.apache.accumulo.server.util.adminCommand;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a fan of mixed case in package names. How about this instead?

Suggested change
package org.apache.accumulo.server.util.adminCommand;
package org.apache.accumulo.server.util.admin.cli;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just following the established convention. ...util.checkCommand, ...util.fateCommand, and ...util.serviceStatus already exist. If we want to change the package names, I think we should do them all together in a different PR.

@dlmarion dlmarion merged commit b49dc0a into apache:main Feb 9, 2026
10 of 16 checks passed
@dlmarion dlmarion deleted the 6086-decompose-admin-command branch February 9, 2026 19:56
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.

accumulo admin and accumulo ec-admin output hard to read and inconsistent

3 participants