Ensure only alive leaders and followers when fetching QQ replica states

[Ayanda-D]

Proposed Changes

This addresses issues reported in #12701 and #10423 where quorum queue leader and follower FSMs have been observed to be down (noproc) while reporting tools such as check_if_node_is_quorum_critical still pass. With this patch, such terminated processes should now be reported by existing CLI tools to help avoid risks of carrying out operations such as node restarts/upgrades, when some QQ members aren't in a good state.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)
• Build system and/or CI

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
If you're unsure about any of them, don't hesitate to ask on the mailing list.
We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• I have added tests that prove my fix is effective or that my feature works
• All tests pass locally with my changes
• If relevant, I have added necessary documentation to https://github.com/rabbitmq/rabbitmq-website
• If relevant, I have added this change to the first version(s) in release-notes that I expect to introduce it

Further Comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution
you did and what alternatives you considered, etc.

[kjnilsson]

whereis/1 will do here, you are already on the node where the process is running.

[Ayanda-D]

Ok, i avoided the rabbit_process:is_process_alive/1 which does the extra distribution work. But also noticed we have rabbit_process:is_registered_process_alive/1 which is already doing whereis/1 with is_pid/1 guard check on the local node, which i'd still have to do on the case statement. seems to be doing same thing (if i'm seeing correct)

[kjnilsson]

case whereis(K) of
    undefined ->
         ...
    Pid -> 
        to_replica_state(T)
end

I'm not sure using a helper function for the above is worth it, just causes read indirection.

[Ayanda-D]

k cool, definitely easier to read. sorry for the back-&-forth on this small item (tendency has been check/use rabbit helpers 1st 😊 after finding myself rewriting some existing utils in the past 😅 ). Yep, the read indirection not necessary here 👍

[kjnilsson]

I'm not sure about returning nil here. is that expected?

[Ayanda-D]

hmm, I think the noproc issue, if my understanding this right, is when we have an entry in ra_state of a member which had already terminated, but wasnt cleaned up from ets from FSM terminate callback. Then its still listed here as if it were still active, yet not alive. But if alive=false, we should handle it as if it had been removed from ra_state, so returning nil here. (i'm not sure if we should also be doing a deletion from ra_state here when the fsm proc is not alive)

[kjnilsson]

all other code paths return a tuple {RegName, Status} so returning [] feels a bit odd, could you either return {K, noproc} or use lists:filtermap to remove the entry all together?

[Ayanda-D]

Ok, lists:filtermap sounds good. this was removing/flattening out the [] on the conversion to a map, but no need for that extra step. updated 👍