Skip to content

Stop MAC servers in parallel - #6575

Open
DomGarguilo wants to merge 1 commit into
apache:2.1from
DomGarguilo:ParallelstopProcs
Open

DomGarguilo wants to merge 1 commit into
apache:2.1from
DomGarguilo:ParallelstopProcs

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

Looking through code paths that could help speed up the ITs and found this one.

In the old code, MiniAccumuloClusterControl.stop() stops tservers, sservers and compactors one at a time and waits after stopping each. The new code now sends the SIGTERM to all processes of that server type first, then waits on each. This allows them all to start shutting down in parallel before the wait step.

I did some rough testing by timing stop() and found that this speeds things up by ~350ms per server which is negligible on a single run but the time savings multiply.

@DomGarguilo DomGarguilo added this to the 2.1.7 milestone Oct 1, 2026
@DomGarguilo DomGarguilo self-assigned this Oct 1, 2026
@dlmarion

dlmarion commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

It looks like we are waiting up to 30 seconds per server even with your change.

            for (Process tserver : tabletServerProcesses) {
              try {
                cluster.stopProcessWithTimeout(tserver, 30, TimeUnit.SECONDS);
              } catch (ExecutionException | TimeoutException e) {
             ...
           }

I wonder if instead we change the signature of cluster.stopProcessWithTimeout to accept all processes, incorporate your change in the that method, and wait a total of 30s for all processes to stop instead of one.

Thoughts?

@DomGarguilo

DomGarguilo commented Oct 1, 2026 •

Copy link
Copy Markdown
Member Author

Yea I think there are some options here. The changes here keep the same order in which servers are shut down. We could try shutting all processes down at once but that would obviously not guarantee shutdown order which might be needed for some reason in certain tests.

Its worth mentioning that the 30s wait is just a worst case scenario but it is per process so if all processes fail to stop properly we would be waiting 30s for each.

@dlmarion

dlmarion commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

The changes here keep the same order in which servers are shut down. We could try shutting all processes down at once but that would obviously not guarantee shutdown order which might be needed for some reason in certain tests.

I'm not sure I'm following w/r/t the order. The changes are in a switch statement based on the server method parameter. So, in this method, we are only shutting down one type of server. I'm just suggesting that instead of this method calling cluster.stopProcessWithTimeout(tserver, 30, TimeUnit.SECONDS); it calls cluster.stopProcessWithTimeout(tabletServerProcesses, 30, TimeUnit.SECONDS); instead and inside of stopProcessWithTimeout we shut them all down in parallel.

@DomGarguilo

Copy link
Copy Markdown
Member Author

I'm not sure I'm following w/r/t the order. The changes are in a switch statement based on the server method parameter. So, in this method, we are only shutting down one type of server. I'm just suggesting that instead of this method calling cluster.stopProcessWithTimeout(tserver, 30, TimeUnit.SECONDS); it calls cluster.stopProcessWithTimeout(tabletServerProcesses, 30, TimeUnit.SECONDS); instead and inside of stopProcessWithTimeout we shut them all down in parallel.

Yea you're right. I was conflating this with stopping all server types at once. Within a single stop() call, order shouldnt matter. It looks like that is how it is in main already where stopProcessesWithTimeout() waits 30s per server type in total which makes more sense.

This branch has not been deployed

No deployments
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.

3 participants