Feat[BMQ, MQB]: shutdown v2, optimizing shutdown logic

[dorjesinpo]

Changing shutdown logic to minimize control plane traffic.

1. StopRequest is now per App (not Cluster)
2. Downstreams of the the shutting down do not deconfigure queues. Upstreams do.
3. The shutting down waits for unconfirmed (not the downstreams)

DownStream              Shutting down broker:           Upstream

                            InitiateShutdown:
                
                            ClientSessions:
                                changes the state
                    
                            Broker sessions:
                    <--         StopRequest V2          -->

Clusters                                                    Clusters
and                                                             The ClusterNodeSession:
ClusterProxies (CQH):                                               QueueHandles:
    Queues:                                                             deconfigureAll
        buffer PUTs
                                                            ClusterProxies:
                                                                The ClientSession:
                                                                    QueueHandles:
                                                                        deconfigureAll
                    -->         StopResponse V2         <--
            
                            Clusters:
                                stopPushing
                                checkUnconfirmedV2
                        
                                continueShutdownDispatched
                                    change the state(s)
                                    cancelAllRequests
                                    delete (dirty) queues

[bmq-oss-ci]

Build 180 of commit e81caf3 has completed with FAILURE

[pniedzielski]

Nothing big. A few questions inline, a pervasive typo, and some comment/docs changes.

Overall comments

• Once we remove the v1 shutdown code, this will be a bit simpler. I think then it might be worth taking a little time to look at simplifying the structure of some of the parts that we've touched here (the separation of labor between initializeShutdown vs stop in mqba_application won't really make sense at that stage).

• Now that we're open-source, I think we need be a bit more careful about what versions of brokers users may try to run. I noted that already we might want to turn off support for ungraceful shutdown, since all OSS brokers support it. But I think we should be intentional about turning off graceful shutdown v1, document when we do it, and provide an upgrade process for this (in the manner of https://kafka.apache.org/documentation/#upgrade).

• I found this comment in mqbblp_clusterproxy's ClusterProxy::stopDispatched:

  // TBD:  In the future, we should review the entire shutdown logic, and
  //       eventually implement a request/response based mechanism in
  //       order to provide true graceful shutdown.
  

  A search revealed that this indeed was from before graceful shutdown logic v1, and from what I understand about this part of the code, this comment can safely be removed. I think changing some of the // temporary comments to something greppable as I noted below will mitigate things like this being left behind from this PR.

Merging

I think we'll need to unrevert Luke's admin API changes before we merge this in. So either: you can make the changes I suggest here and I'll approve before we unrevert the changes, and I can approve again after you rebase, or you can hold off on making the changes until after we unvert those changes and I'll approve then.

[pniedzielski]

The post-conditions of QueueEngineUtil::AppState::reset are a little too intricate for my tastes; reset doesn't actually reset everything about the AppState... These two lines look like they should be reversed based on their names, but no, they are correct. I skimmed through the uses of reset and it's almost always followed by modifying d_routing_sp in some way.

Nothing to change in your PR about this, but I've written this down as something to revisit.

[dorjesinpo]

Should we change it? How about undoRouting?

[pniedzielski]

undoRouting is a good name for this. If you want to make that change in this PR or later, either way is okay by me. Low priority.

EDIT I see you made the change, looks good.

[bmq-oss-ci]

Build 230 of commit 711cad0 has completed with FAILURE

[pniedzielski]

👍🏻

Let me know if you need a re-approval after fixing merge conflicts for getting this into main.

[pniedzielski]

Reapproving after rebase to fix merge conflicts. Spot checked that no accidental changes were made.

[bmq-oss-ci]

Build 247 of commit b28cb4f has completed with FAILURE

[bmq-oss-ci]

Build 260 of commit c5b7c31 has completed with FAILURE

[bmq-oss-ci]

Build 267 of commit 670dde5 has completed with FAILURE