Instance check leads to UnreachableMaster (LastCheckValid: false) if instance check takes longer than 1s

[binwiederhier]

Our MySQL hosts are under considerable load, to the point that quite frequently, the analysis returns LastCheckValid: false, even though the host is up and responsive:

analysis: ClusterName: ...:3306, IsMaster: true, LastCheckValid: false, LastCheckPartialSuccess: true, 
CountReplicas: 2, CountValidReplicas: 2, CountValidReplicatingReplicas: 2, CountLaggingReplicas: 0, 
CountDelayedReplicas: 0, CountReplicasFailingToConnectToMaster: 0

After looking at Wireshark dumps, it seems that our hosts sometimes take slightly longer than 1s to perform these checks (2-3s, sometimes even longer), which is arguably not great, but still ok for us. From my understanding of the code, if a check takes longer than 1s (hardcoded), Orchestrator considers the host to be down, which may lead to emergent actions and eventually to failovers if those fail.

I've deduced this from these parts of the code:

• https://github.com/openark/orchestrator/blob/v3.2.3/go/inst/analysis.go#L222-L226
• https://github.com/openark/orchestrator/blob/v3.2.3/go/inst/analysis_dao.go#L159-L162

I propose to making this 1s timeout configurable as ReasonableInstanceCheckSeconds.

Did I get all of this right? What do you think about the proposal? I am happy to provide all the info that's needed here.

[shlomi-noach]

This makes sense. I remember going to opposite direction, this used to be configurable and moved to a constant in an attempt to reduce variance and configuration complexity. But honestly my memory may not serve me well. Thank you for submitting a PR!

[binwiederhier]

Closing this as the PR is merged.