Made partition move cancellations more robust

[mmaslankaprv]

Problem statement

Since reverting partition configuration involves a genuine Raft group reconfiguration it is vulnerable to all the same issues that may prevent raft reconfiguration from finishing.

Since the replica set of partition was already updated in the topic table the controller backend reconciliation loop would wait for the reconfiguration to finish and as the movement was already cancelled it can not be cancelled again.

Example

Consider a situation where in a 4-nodes cluster containing nodes (0,1,2,3) a partition is being moved from replica set (0,1,2) to (1,2,3). In this case a node 3 is added to the replica set while node 0 is removed. When configuration change is requested Raft configuration is going through the set of changes as described in [1] if cancel is requested after raft reconfiguration to (1,2,3) finished but before the finish_moving_replicas command was replicated controller backend will instruct raft to revert configuration back to (0,1,2). The first step involving change from (1,2,3) to (0,1,2) is adding node 0 as a learner to the current configuration i.e. there will be one learner - 0. If node 0 is unavailable the raft reconfiguration will never finish as node 0 will never be promoted to the voter.

Solution

Added handling for revert_cancel_partition_move command to the topic table. The command is going to be emitted by controller_backend when partition underlying raft group reconfiguration that is requested to be canceled is already finished. The command finishes move and updates the topic partition with replica set aligned with the target of original
move command.

Partition movement state transitions:

When partition is being moved from replica set A to replica set B it may
either be finished or cancelled. The cancellation on the other hand may be
reverted. The partition movement state transitions are explained in the
following diagram:

 A - replica set A
 B - replica set B

 ┌──────────┐   move           ┌──────────┐       finish         ┌──────────┐
 │  static  │                  │  moving  │                      │  static  │
 │          ├─────────────────►│          ├─────────────────────►│          │
 │    A     │                  │  A -> B  │                      │    B     │
 └──────────┘                  └────┬─────┘                      └──────────┘
      ▲                             │                                  ▲
      │                             │                                  │
      │                             │ cancel/force_cancel              │
      │                             │                                  │
      │                             │                                  │
      │                             ▼                                  │
      │                        ┌──────────┐                            │
      │        finish          │  moving  │     revert_cancel          │
      └────────────────────────┤          ├────────────────────────────┘
                               │  B -> A  │
                               └──────────┴─────┐
                                    ▲           │
                                    │           │
                                    │           │
                                    │           │
                                    └───────────┘
                                        force_cancel

Backports Required

• none - not a bug fix
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v22.3.x
• v22.2.x
• v22.1.x

UX Changes

Release Notes

Improvements

• more robust partition movements related with decommissioning/recommissioning nodes that are unavailable

[ztlpn]

I wonder if we can repurpose the finish command for revert_cancel? I.e. if we are in the moving(B -> A) state, revert_cancel becomes finish(B).

[mmaslankaprv]

I was considering this, but when it comes to writing the code it would be much more complicated. Handling of finish command already is pretty complex and i didn't want to add to it. Given we introduce the feature in the feature table adding a new command is not really an effort and i think it makes handling a little bit easier.

[ztlpn]

so the difference between the code in this branch and update_partition_replica_set is that the latter is wrapped in apply_configuration_change_on_leader, but why do we do it in the force-abort case and not in the soft cancel case?

[mmaslankaprv]

Whole cancel_replica_set_update is executed on the leader. Force cancellation should work even when there is no leader. But when configuration change already finished there must be a leader as it was successful, hence we dispatch this last update piece on the leader

[bharathv]

lgtm, just catching up on reviews.