Skip to content

2025.1.5.0-b84

2025.1.5.0-b84: [BACKPORT 2025.1][#31930,#31241] docdb: Fix RBS vs ConfigChange races
@ttyusupov ttyusupov tagged this 02 Jul 16:02 
Summary:
There is a possibility of RBS vs ConfigChange races, for example:
1. Raft config for the tablet has nodes A (leader), B, C.
2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C
3. RBS A -> D started
4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader
5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config

**Expected result:** orphaned tablet replica on D should be deleted.
**Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader.

In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group.

The following logic is implemented to support that:
1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending.
2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted).
3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2).
4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both:

  a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet.

  b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient.
5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart).

Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them.

Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change.

Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field.

**Upgrade/Rollback safety:**
New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field.

Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759

Test Plan:
Run the following tests for asan/tsan/debug/relelase for 20 iterations each:
- TabletSplitITest.SplitWithParentTabletRbsFromFollower
- TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace
- TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart
- TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241
- TabletReplacementITest.DontDeleteNewReplicaInPendingConfig
- TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5

Reviewers: zdrudi

Reviewed By: zdrudi

Subscribers: ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D55088
Assets 2
Loading