Handle failures in GracefulMasterTakeover

[o-fedorov]

Related issue: #43

Description

1. If GracefulMasterTakeover fails, reset read_only for master.
2. Introduce PostUnsuccessfulGracefulTakeoverProcesses to allow running more sophisticated recovery.
3. Add a related system test.

[o-fedorov]

@kamil-holubicki, could I please ask you to take a look at this PR?

[jim3ccc]

LGTM

[o-fedorov]

Hello @kamil-holubicki. Hope you are doing well. Could you please share any feedback regarding this PR?

[o-fedorov]

@egegunes, @fabio-silva, @kamil-holubicki, @igroene, could I please ask you for a feedback regarding this PR?

[hulyav]

Unfortunately, we do not prioritize feature requests or code submissions with our limited bandwidth for Orchestrator as our focus is on more widely impacting issues that affect the majority of the Percona user base. I understand there are discussions underway between our sales team and your leadership that might allow us to one-day take this on.

[o-fedorov]

Hi @hulyav. Thank you for your response. I’d like to highlight that my team has already fixed this particular bug on our side, and merging the fix to the upstream would benefit the community, since there can be other users that face this issue. Also, it would be nice to merge this pull request sooner rather than later to prevent the codebases divergence.

[o-fedorov]

Hello @hulyav and @kamil-holubicki . It is already 4 month since this PR was created. I have a feeling that it eventually will not be accepted. To get rid of the uncertainty about this change, should I close this PR and the corresponding issue as rejected?

[kamil-holubicki]

Hi @o-fedorov , I'm not sure about this error propagation and handling. In the original flow, if ForceExecuteRecovery returned err, but also returned recoveryAttempted and topologyRecovery, GracefulMasterTakeover continued.
Now it makes master R/W (which is fine), but then returns.

[o-fedorov]

Thank you, @kamil-holubicki , I forgot to clear the error at the end of the function. Updated now.

Note that in the original code, err is ignored below the refactored part, and is eventually overridden.

[hulyav]

Thanks for submitting the patch — we appreciate the effort. While we do maintain a fork of Orchestrator for use in our own products, we aren’t the upstream maintainers and generally don’t make changes unless it addresses an issue we’re actively working on or a customer has commissioned the fix. That said, you're welcome to keep the PR open or close it at your discretion — totally your call.