Skip to content

Conversation

@vazois
Copy link
Collaborator

@vazois vazois commented Mar 11, 2025

This PR adds two new informational fields related to failover and recovery status.
In addition, fixes a race condition when acquiring the recovery lock.

@vazois vazois marked this pull request as ready for review March 11, 2025 20:34
Copilot AI review requested due to automatic review settings March 11, 2025 20:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@vazois vazois force-pushed the vazois/expose-recovery branch 4 times, most recently from d22bcc1 to 9400006 Compare March 12, 2025 18:18
@badrishc badrishc requested a review from Copilot March 12, 2025 19:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR exposes additional recovery and failover status information by introducing new enum values for recovery status and updating related logging and error handling. It also addresses a race condition when acquiring the recovery lock by adding a parameterized StartRecovery method.

  • Adds a new RecoveryStatus enum with states for various recovery phases.
  • Updates FailoverStatus and its usage in logs and metrics.
  • Modifies ReplicationManager, FailoverManager, and other related modules to use the new status values.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
libs/cluster/Server/Replication/RecoveryStatus.cs Introduces the RecoveryStatus enum with new statuses, including minor comment typos.
libs/cluster/Server/Failover/FailoverStatus.cs Adds new failover statuses and updates the status mapping in GetFailoverStatus.
libs/cluster/Server/Replication/ReplicationManager.cs Updates StartRecovery to accept a RecoveryStatus parameter and logs the status accordingly.
libs/cluster/Server/Failover/FailoverManager.cs Exposes a new lastFailoverStatus field and adds a getter for it.
libs/cluster/Server/ClusterProvider.cs Adds recovery and failover metrics via the updated replication info.
libs/cluster/Server/Failover/ReplicaFailoverSession.cs Updates the call to StartRecovery with a specific RecoveryStatus value and controls the recovery lock release accordingly.
libs/cluster/Server/ClusterManagerWorkerState.cs Modifies the StartRecovery call to use a specific failure recovery status.
libs/cluster/Session/ReplicaOfCommand.cs Adjusts the call to StartRecovery using the appropriate RecoveryStatus.

@vazois vazois force-pushed the vazois/expose-recovery branch 2 times, most recently from c416eb2 to 02525f8 Compare March 12, 2025 20:48
@TalZaccai TalZaccai requested a review from badrishc March 13, 2025 18:32
@vazois vazois force-pushed the vazois/expose-recovery branch from 22d30dc to 011c5ea Compare March 14, 2025 18:46
@vazois vazois merged commit 3ba2785 into microsoft:main Mar 14, 2025
26 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 14, 2025
@vazois vazois deleted the vazois/expose-recovery branch June 26, 2025 16:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants