Skip to content

Commit 43800d4

Browse files
johscheuerjohscheuer
authored
Add design for better coordination for multi operator setup (#2172)
* Add design for better coordination for multi operator setup
1 parent 9584d12 commit 43800d4

File tree

1 file changed

+265
-0
lines changed

1 file changed

+265
-0
lines changed

‎docs/design/better_coordination_multi_operator.md‎

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,265 @@
1+
# Better coordination for multiple operator instances
2+
3+
## Metadata
4+
5+
* Authors: @johscheuer
6+
* Created: 2024-11-19
7+
* Updated: 2024-11-19
8+
9+
## Background
10+
11+
The current way to run multi-region FoundationDB deployments with the operator is to run multiple operator instances in different namespaces and/or in different Kubernetes clusters.
12+
This brings a few synchronization challenges as those different operator instances are not communicating with each other and they are not sharing state.
13+
We already added the `LockClient` to synchronize the upgrade step for minor and major upgrades, where all processes must be restarted at once.
14+
The `LockClient` is also used to get a lease that allows the operator instance to perform certain actions, like exclusions, that should only be executed by one instance at a time.
15+
16+
The difference between the upgrades and the other operations that should be coordinated is that the upgrades have a "global" source of truth.
17+
During the upgrades the each operator instance can check that all processes in the cluster are ready for being upgraded.
18+
In contrast the other operations are lacking such a "global" source of truth and only have their own local state based on th `FoundationDBCluster` resource.
19+
Adding an optimistic coordination mechanism doesn't guarantee that the operations are executed once but it will increase the probability that the operator instance have enough time to coordinate.
20+
Most operations have some requirements before they can be executed, e.g. in case of a knob rollout the `ConfigMap` must be updated and synced to all affected Pods or in the case of a replacement a new pod and the according resources must be created.
21+
The waiting time until the prerequisites are satisfied should be enough time to allow the individual operator instances to update the pending list for the coordination.
22+
This is still an imperfect solution which will not guarantee that the operator instances will coordinate, but the solution is a good enough solution until we have a multi-cluster operator.
23+
In order to increase the probability that the different operator instances have time to update the pending list, we could add a dedicated wait window until an operation will be executed.
24+
Adding a wait window will have the side effect that the rollout of a change might take longer than required.
25+
26+
One assumption for the coordination is, that all operations that could use coordination will update the `FoundationDBCluster` resources in a short time window, e.g. by the same deploy pipeline.
27+
28+
29+
## General Design Goals
30+
31+
This design will show a possible way to synchronize the `exclusion`, `inclusion` and `removal` of process groups across different operator instances for the multi-region setup.
32+
33+
## Current Implementation
34+
35+
Right now only the restart (bounce) of the processes during a minor or major version upgrade is coordinated.
36+
All other bounces, exclusions, inclusions or removals are not coordinated with the locking mechanism only one operator instance will perform the according action to it's "local" processes.
37+
In most cases this works fine but causes more disruptions than required as every operator instance is perform the same set of operations on the local set of processes.
38+
The missing synchronization between the operator instance can result in multiple coordinator changes during the rollout of a setting or during an upgrade.
39+
40+
## Proposed Design
41+
42+
The idea is to extend the existing `AdminClient` with some similar functionality from the `LockClient` to coordinate the following actions:
43+
44+
- coordinator selection
45+
- exclusions
46+
- inclusions
47+
- bounces
48+
49+
The following methods will be added:
50+
51+
- `UpdatePendingForRemoval`: Updates the set of process groups that are marked for removal, an update can be eiter the addition or removal of a process group.
52+
- `UpdatePendingForExclusion`: Updates the set of process groups that should be excluded, an update can be eiter the addition or removal of a process group.
53+
- `UpdatePendingForInclusion`: Updates the set of process groups that should be included, an update can be eiter the addition or removal of a process group.
54+
- `UpdatePendingForRestart`: Updates the set of process groups that should be restarted, an update can be eiter the addition or removal of a process group.
55+
- `UpdateReadyForExclusion`: Updates the set of process groups that are ready to be excluded, an update can be eiter the addition or removal of a process group
56+
- `UpdateReadyForInclusion`: Updates the set of process groups that are ready to be included, an update can be eiter the addition or removal of a process group.
57+
- `UpdateReadyForRestart`: Updates the set of process groups that are ready to be restarted, an update can be eiter the addition or removal of a process group
58+
- `GetPendingForRemoval`: Gets the process group IDs for all process groups that are marked for removal.
59+
- `GetPendingForExclusion`: Gets the process group IDs for all process groups that should be excluded.
60+
- `GetPendingForInclusion`: Gets the process group IDs for all the process groups that should be included.
61+
- `GetPendingForRestart`: Gets the process group IDs for all the process groups that should be restarted.
62+
- `GetReadyForExclusion`: Gets the process group IDs for all the process groups that are ready to be excluded.
63+
- `GetReadyForInclusion`: Gets the process group IDs for all the process groups that are ready to be included.
64+
- `GetReadyForRestart`: Gets the process group IDs fir akk tge process groups that are ready to be restarted.
65+
66+
The following sub-reconciler will be added or modified:
67+
68+
- `operatorCoordination`: Will add process groups that are marked for removal to the key range and removes them from the key range if they are not being present anymore in the `FoundationDBCluster` status and the process group prefix matches. This sub-reconciler will also remove process groups from the `readyForExclusion`, `readyForInclusion` and `readyForRestart` (and the same for pending).
69+
- `excludeProcesses`: Will add process groups that are pending for exclusions and add process groups that are ready for exclusion.
70+
- `changeCoordinators`: Will read the `pendingForRemoval` set and will not take any process group from this set as a candidate for the new coordinators.
71+
- `removeProcessGroups`: Will add process groups that can be included to `readyForInclusion`.
72+
73+
We will add a new setting under the `FoundationDBClusterAutomationOptions` with the name `synronizationMode`.
74+
The default will be `local`, which will represent the current state and the new mechanism can be enabled by setting the value to `global`.
75+
Those changes will add some additional load to the cluster, but the additional load should be fairly limited compared to the customer load on those clusters and the request pattern will be mostly reading the data.
76+
77+
### FoundationDB Key Space
78+
79+
The information about pending and ready processes will be stored in FDB itself, so it will be available for all operators and will get the same benefits from the transaction guarantees.
80+
The keys will be prefixed with the `client.cluster.GetLockPrefix()` (default `\xff\xff/org.foundationdb.kubernetes-operator`) and the according path, e.g. `readyForExclusion`.
81+
The value will be empty, except for the `readyForExclusion` and `readyForInclusion` case, in those cases the value will be the process group address(es) that should be used for exclusion or inclusion.
82+
83+
For efficient scans we will add a sub-path between the prefix and the process group id with the process group id prefix.
84+
85+
Example: The resulting key-value mappings for the process group id `kube-cluster-1-storage-1` (`kube-cluster-1` is the process group id prefix) with locality based exclusions enabled will look like this for the exclusion case:
86+
87+
```text
88+
# This key will be directly added when the process group is marked for removal.
89+
\xff\xff/org.foundationdb.kubernetes-operator/pendingForExclusion/kube-cluster-1/kube-cluster-1-storage-1 - {}
90+
# This key will be added by the exclusion sub-reconciler, as soon as the exclusion could be executed.
91+
\xff\xff/org.foundationdb.kubernetes-operator/readyForExclusion/kube-cluster-1/kube-cluster-1-storage-1 - locality_instance_id:kube-cluster-1-storage-1
92+
```
93+
94+
Each operator instance will only write or delete the "local" process groups.
95+
"Local" means in this context that the operator instance only writes into the sub-path that has the `processGroupIDPrefix` which is defined in the `FoundationDBCluster` resource.
96+
The operator instance will read the data from all process groups to.
97+
The reading and writing will be handled in a transaction to ensure either all keys are updated or none.
98+
99+
In the case that the `processGroupIDPrefix` should change, the operator will migrate all the "old" keys to the desired keys in a single transaction.
100+
101+
Example: The above `processGroupIDPrefix` changes from `kube-cluster-1` to `unicorn`:
102+
103+
```text
104+
\xff\xff/org.foundationdb.kubernetes-operator/pendingForExclusion/kube-cluster-1/kube-cluster-1-storage-1 --> \xff\xff/org.foundationdb.kubernetes-operator/pendingForExclusion/unicorn/kube-cluster-1-storage-1
105+
\xff\xff/org.foundationdb.kubernetes-operator/readyForExclusion/kube-cluster-1/kube-cluster-1-storage-1 --> \xff\xff/org.foundationdb.kubernetes-operator/readyForExclusion/unicorn/kube-cluster-1-storage-1
106+
```
107+
108+
Once the required action was executed or the process groups are not being part of the cluster anymore the entries will be removed.
109+
The `UpdateStatus` sub-reconciler will perform checks to remove old entries.
110+
111+
### Example Exclusion Sub-Reconciler
112+
113+
The following example will show the required modifications for the `excludeProcesses` sub-reconciler.
114+
115+
1. The operator instance fetches all processes that are pending for exclusion from the `\xff\xff/org.foundationdb.kubernetes-operator/pendingForExclusion` key space.
116+
2. The operator instance will filter out all local process groups that must be excluded but are missing in the `pendingForExclusion` key space and adds them.
117+
3. In the next step the operator performs the same steps to check if the exclusions could be done.
118+
4. If the local exclusions are allowed, the operator will add all the processes that can be excluded to the `\xff\xff/org.foundationdb.kubernetes-operator/readyForExclusion` key space.
119+
5. Now the operator checks if all the entries from `pendingForExclusion` are also present in `readyForExclusion`, this check should be done on a process class basis.
120+
6. If the two sets are coherent (contain the exact same elements), the operator will perform the usual exclusion steps, e.g. get a lock before issuing the exclude command.
121+
122+
In the initial implementation we are not adding any dedicated wait steps to increase the probability that each operator instance can add its entries to the key space.
123+
The assumption here is that all operations have some prerequisites before they can be executed and in most cases waiting for the prerequisites should give the operator instances enough time to add the process groups to the pending list(s).
124+
125+
```go
126+
func (e excludeProcesses) reconcile(ctx context.Context, r *FoundationDBClusterReconciler, cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta2.FoundationDBStatus, logger logr.Logger) *requeue {
127+
// Check which processes can be excluded and if it's safe to exclude processes.
128+
// ...
129+
// In case that there are processes from different transaction process classes, we expect that the operator is allowed
130+
// to exclude processes from all the different process classes. If not the operator will delay the exclusion.
131+
if !transactionSystemExclusionAllowed {
132+
return &requeue{
133+
message: "more exclusions needed but not allowed, have to wait until new processes for the transaction system are up to reduce number of recoveries.",
134+
delayedRequeue: true,
135+
}
136+
}
137+
138+
pendingForExclusions, err := adminClient.GetPendingForExclusion()
139+
// error handling
140+
readyForExclusion, err := adminClient.GetReadyForExclusion()
141+
// error handling
142+
// Add the new process groups that are ready to be excluded.
143+
err := adminClient.UpdateReadyForExclusion(missingFdbProcessesReadyToExclude)
144+
// error handling
145+
146+
// Ensure that pendingForExclusions includes all the process groups (also the newly added once).
147+
// Check if readyForExclusion contains all the process groups from pendingForExclusions if so proceed with the exclusion, if not
148+
// do a delayed requeue but don't issue the exclusion.
149+
if !equality.Semantic.DeepEqual(readyForExclusion, pendingForExclusions) {
150+
return &requeue{
151+
message: fmt.Sprintf("more processes are pending exclusions, will wait until they are ready to be excluded %d/%d", len(readyForExclusion), len(pendingForExclusions)),
152+
delayedRequeue: true,
153+
}
154+
}
155+
156+
// If a coordinator should be excluded, we will change the coordinators before doing the exclusion. This should reduce the
157+
// observed recoveries, see: https://github.com/FoundationDB/fdb-kubernetes-operator/issues/2018.
158+
if coordinatorExcluded {
159+
coordinatorErr = coordinator.ChangeCoordinators(logger, adminClient, cluster, status)
160+
}
161+
162+
// Perform exclusion for all the readyForExclusion addresses
163+
164+
return nil
165+
}
166+
```
167+
168+
### Operator Coordination Sub-Reconciler
169+
170+
This new sub-reconciler will run after the first `updateStatus` sub-reconciler and will be responsible for adding and removing process groups from the state in the FoundationDBCluster.
171+
For this the operator will iterate over all process groups from the `FoundationDBCluster` resource.
172+
Since this sub-reconciler runs after the first `updateStatus` sub-reconciler it should have the latest information.
173+
174+
When a process group is marked for removal and not excluded the operator will add this process group to the set of `pendingForRemoval`, `pendingForExclusion` and `pendingForInclusion`.
175+
When a process group has the `IncorrectCommandLine` condition it will be added to `pendingForRestart`.
176+
When a process group has a non-nil `ExclusionTimestamp` it will be removed from `pendingForExclusion` and `readyForExclusion` as the process group was excluded.
177+
When a process group is removed all associated entries will be removed too.
178+
179+
If the `synronizationMode` is set to `local` the operator will skip any work and any data that is still present must be deleted manually.
180+
Otherwise the sub-reconciler would need to always read the key ranges which could have an affect to existing FoundationDB clusters.
181+
182+
The goal of this new sub-reconciler is to manage the state in FoundationDB for the local process groups.
183+
Adding a new sub-reconciler will be easier to maintain instead of adding this logic to the `updateStatus` sub-reconciler.
184+
185+
```go
186+
func (reconciler operatorCoordination) reconcile(ctx context.Context, r *FoundationDBClusterReconciler, cluster *fdbv1beta2.FoundationDBCluster, _ *fdbv1beta2.FoundationDBStatus, logger logr.Logger) *requeue {
187+
// If the synchronization mode is local (default) skip all work
188+
if cluster.GetSynronizationMode() == string(fdbv1beta2.SynronizationModeLocal) {
189+
return nil
190+
}
191+
192+
// Create an admin client to interact with the FoundationDB cluster
193+
adminClient, err := r.getAdminClient(logger, cluster)
194+
if err != nil {
195+
return &requeue{curError: err}
196+
}
197+
defer adminClient.Close()
198+
199+
// Read all data from the lists to get the current state. If a prefix is provided to the get methods, only
200+
// process groups with the additional sub path will be returned.
201+
pendingForExclusion, err := adminClient.GetPendingForExclusion(cluster.Spec.ProcessGroupPrefix)
202+
// error handling
203+
// repeat for all get methods
204+
205+
// UpdateAction can be "delete" or "add". If the action is "add" the entry will be added, if
206+
// the action is "delete" the entry will be deleted.
207+
updatesPendingForExclusion := map[fdbv1beta2.ProcessGroupID]fdbv1beta2.UpdateAction{}
208+
209+
// Iterate over all process groups to generate the expected state.
210+
for _, processGroup := range cluster.Status.ProcessGroups {
211+
// Keep track of the visited process group to remove entries from removed process groups.
212+
visited[processGroup.ProcessGroupID] = fdbv1beta2.None{}
213+
if processGroup.IsMarkedForRemoval() && !processGroup.IsExcluded() {
214+
// Check if process group is present in pendingForRemoval, pendingForExclusion and pendingForInclusion.
215+
// If not add it to the according set.
216+
// ...
217+
218+
// Will be repeated for the other fields.
219+
if _, ok := pendingForExclusion[processGroup.ProcessGroupID]; !ok {
220+
updatesPendingForExclusion[processGroup.ProcessGroupID] = fdbv1beta2.UpdateActionAdd
221+
}
222+
}
223+
224+
if processGroup.GetConditionTime(fdbv1beta2.IncorrectCommandLine) != nil {
225+
// Check if the process group is present in pendingForRestart.
226+
// If not add it to the according set.
227+
// ...
228+
} else {
229+
// Check if the process group is present in pendingForRestart or readyForRestart.
230+
// If so, add them to the set to remove those entries as the process has the correct command line.
231+
// ...
232+
}
233+
234+
if processGroup.IsExcluded() {
235+
// Check if the process group is present in pendingForExclusion or readyForExclusion.
236+
// If so, add them to the set to remove those entries as the process is already excluded.
237+
// ...
238+
if _, ok := pendingForExclusion[processGroup.ProcessGroupID]; ok {
239+
updatesPendingForExclusion[processGroup.ProcessGroupID] = fdbv1beta2.UpdateActionDelete
240+
}
241+
}
242+
}
243+
244+
// Iterate over all the sets and mark all entries that are associated with a removed process group to be
245+
// removed.
246+
for _, processGroup :- range updatesPendingForExclusion {
247+
// If the process group was not visited the process group was removed and all the
248+
// associated entries should be removed too.
249+
if _, ok := visited[processGroup.ProcessGroupID]; !ok {
250+
updatesPendingForExclusion[processGroup.ProcessGroupID] = fdbv1beta2.UpdateActionDelete
251+
}
252+
}
253+
254+
err = adminClient.UpdatePendingForExclusion(updatesPendingForExclusion)
255+
// error handling
256+
257+
return nil
258+
}
259+
```
260+
261+
For the new setting we will be adding a new dedicated e2e test suite to ensure all changes are properly tested.
262+
263+
## Related Links
264+
265+
-

0 commit comments

Comments
 (0)