Skip to content

fix(drain): use CAS-fenced RuleOperation requests when removing replicas #576

Description

@Verolop

Background

PR #575 upgrades the Multigres dependency and runtime images to 3533683.

Upstream renamed the protobuf operation used when removing a replica:

  • Old: CohortUpdateOperation_COHORT_UPDATE_OPERATION_REMOVE
  • New: RuleOperation_RULE_OPERATION_COHORT_REMOVE

The operator uses the old name in two places in pkg/data-handler/drain/drain.go, so the project no longer compiles after the dependency update.

There is also a pre-existing runtime bug that is live today, independent of the upgrade. UpdateConsensusRule requires expected_outgoing_rule, but the operator does not send it. The compare-and-swap guard is already enforced at the currently pinned 7edf67aa, so a healthy multipooler rejects every operator-issued replica-removal request with:

expected_outgoing_rule is required (compare-and-swap guard)

That leaves the pod retrying its drain until the five-minute force-unregister timeout, which masks the failure rather than surfacing it.

This is an interim correctness fix for the current drain implementation. The longer-term replacement with the upstream Drain() RPC remains tracked in #435.

Required changes

  • Replace both obsolete enum references with RuleOperation_RULE_OPERATION_COHORT_REMOVE.
  • Before removing a standby, call Status on the currently selected primary.
  • Read the primary's current decided consensus rule from the status response.
  • Put that rule number in UpdateConsensusRuleRequest.ExpectedOutgoingRule.
  • Do not send an update if the status response is missing consensus information or contains an undecided proposal.
  • If the status read fails, the primary changes, or the CAS is rejected, leave the pod in its current drain state and requeue it.
  • Fetch a fresh rule on the next reconciliation instead of retrying with a stale value.
  • Preserve the existing five-minute emergency force-unregister behavior.
  • Factor the duplicated standby-removal logic into one helper used by both the Requested and Draining states.
  • Decide whether CoordinatorId should identify the operator. If it remains unset, document that the multipooler's server-side default is intentional.

Acceptance criteria

  • The operator compiles against multigres@3533683.
  • Both drain states send RULE_OPERATION_COHORT_REMOVE.
  • Every removal request contains the target standby ID and a fresh ExpectedOutgoingRule.
  • A missing, malformed, or undecided consensus status causes a safe retry.
  • A stale-rule/CAS rejection causes a safe retry with a fresh status on the next reconciliation.
  • Repeating removal after the standby is already absent succeeds idempotently.
  • The pod does not advance to the next drain state when the status or update RPC fails.
  • Existing primary-unavailable and force-unregister behavior remains covered.
  • make lint, make build, make test-integration, and the regular unit tests pass.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions