Soft-leader election mechanism for cc based on observed cluster state #2252

muhamadazmy · 2024-11-08T13:12:06Z

Soft-leader election mechanism for cc based on observed cluster state

Stack created with Sapling. Best reviewed with ReviewStack.

muhamadazmy · 2024-11-15T10:11:49Z

crates/admin/src/cluster_controller/service.rs

+                _ = nodes_watcher.changed() => {
+                    // if nodes changes we need to try again in
+                    // our leadership state has changed


While this is not necessary because the cluster_state_watcher will eventually unlock this loop and hence new 'is_active_controller' will be in effect. But this might react faster to changes in nodes metadata.

What do you think?

tillrohrmann

Thanks for creating this PR @muhamadazmy. The PR is a good improvement. The one thing I'd like to discuss is how we learn about metadata updates when we are not the leader. Either when becoming the leader we need to refresh explicitly or we propagate these changes also when we are a follower. Otherwise we might miss some updates and work on stale information.

tillrohrmann · 2024-11-15T13:59:07Z

crates/admin/src/cluster_controller/service.rs

+                    // if nodes changes we need to try again in
+                    // case our leadership state has changed


What are we doing when the NodesConfiguration changes? Right now, it looks like a no-op.

It's no-op/continue so "known" CC leader can be updated. Since active leader state depends on the nodes config metadata.

I think it's not important since this will be detected by the cluster state updates and hence can be removed

tillrohrmann · 2024-11-15T14:02:59Z

crates/admin/src/cluster_controller/service.rs

+        let maybe_leader = nodes_config
+            .get_admin_nodes()
+            .filter(|node| observed_cluster_state.is_node_alive(node.current_generation))
+            .map(|node| node.current_generation.as_plain())
+            .sorted()
+            .next();
+
+        // assume active if no leader CC (None) or self holds the smallest plain node id with role admin
+        !maybe_leader.is_some_and(|admin_id| admin_id != self.metadata.my_node_id().as_plain())


Instead of computing this repeatedly should we store it and only recompute if either the NodesConfiguration or the ObservedClusterState changes?

We can of course do that. I was thinking it's so cheap to compute based on the current observed cluster state and node config

tillrohrmann · 2024-11-15T14:06:01Z

crates/admin/src/cluster_controller/service.rs

+        let maybe_leader = nodes_config
+            .get_admin_nodes()
+            .filter(|node| observed_cluster_state.is_node_alive(node.current_generation))
+            .map(|node| node.current_generation.as_plain())
+            .sorted()
+            .next();
+
+        // assume active if no leader CC (None) or self holds the smallest plain node id with role admin
+        !maybe_leader.is_some_and(|admin_id| admin_id != self.metadata.my_node_id().as_plain())


Why not comparing against the generational node id? This would potentially guard against an outdated admin node thinking to be the leader.

Ah yes, good catch.

tillrohrmann · 2024-11-15T14:06:43Z

crates/admin/src/cluster_controller/service.rs

@@ -356,6 +365,48 @@ impl<T: TransportConnect> Service<T> {
        }
    }

+    fn is_active_controller(


Maybe align active and leader. If we choose one term then there is only a single concept.

tillrohrmann · 2024-11-15T14:19:38Z

crates/admin/src/cluster_controller/service.rs

+                Ok(_) = logs_watcher.changed(), if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) => {
                    logs_controller.on_logs_update(self.metadata.logs_ref())?;
                    // tell the scheduler about potentially newly provisioned logs
                    scheduler.on_logs_update(logs.live_load(), partition_table.live_load()).await?
                }


What happens in the following case: We are currently not the leader and therefore, ignore the logs updates. Now we become the leader. W/o explicitly updating the Logs configuration, we will operate on old data, right? And if there is no other update, then we won't learn about it.

You are right. I was under the wrong impression that once you are leader you use the latest metadata object. But that is not correct

muhamadazmy · 2024-11-15T15:08:55Z

thank you @tillrohrmann for your review. I will address your comments and get back to you

muhamadazmy · 2024-11-15T16:25:20Z

@tillrohrmann can you give this PR another look. I tried to address all of your comments. Let me know if I still missed other scenarios.

Right now, all CCs will only update their view of the world when either the cluster state or the metadata changes. Only active CC will take action.

tillrohrmann

Thanks for updating the PR @muhamadazmy. I left a few more comments.

tillrohrmann · 2024-11-15T18:01:36Z

crates/admin/src/cluster_controller/service.rs

                }
-                Ok(_) = partition_table_watcher.changed() => {
+                Ok(_) = partition_table_watcher.changed()=> {


Suggested change

Ok(_) = partition_table_watcher.changed()=> {

Ok(_) = partition_table_watcher.changed() => {

tillrohrmann · 2024-11-15T18:02:06Z

crates/admin/src/cluster_controller/service.rs

+                _ = nodes_watcher.changed() => {
+                    // if nodes config has changed we need to take
+                    // action if suddenly we are leader
+                    if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) && !is_active_controller{


Suggested change

if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) && !is_active_controller{

if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) && !is_active_controller {

tillrohrmann · 2024-11-15T18:02:35Z

crates/admin/src/cluster_controller/service.rs

+        let maybe_leader = nodes_config
+            .get_admin_nodes()
+            .filter(|node| observed_cluster_state.is_node_alive(node.current_generation))
+            .map(|node| node.current_generation)
+            .sorted()
+            .next();
+
+        // assume active if no leader CC (None) or self holds the smallest plain node id with role admin
+        !maybe_leader.is_some_and(|admin_id| admin_id != self.metadata.my_node_id())


Here we are still talking about leaders.

tillrohrmann · 2024-11-15T18:03:16Z

crates/admin/src/cluster_controller/service.rs

+        logs_controller: &mut LogsController,
+        scheduler: &mut Scheduler<T>,
+    ) -> anyhow::Result<()> {
+        trace!("Acting like a cluster controller");


Suggested change

trace!("Acting like a cluster controller");

trace!("Acting as the active cluster controller");

?

tillrohrmann · 2024-11-15T18:04:55Z

crates/admin/src/cluster_controller/service.rs

        loop {
            tokio::select! {
                _ = self.heartbeat_interval.tick() => {
                    // Ignore error if system is shutting down
                    let _ = self.cluster_state_refresher.schedule_refresh();
+                    trace!("Is active cluster controller leader: {is_active_controller}");


Not sure whether this log line is really helpful. Especially since we are logging the same on line 323.

tillrohrmann · 2024-11-15T18:05:38Z

crates/admin/src/cluster_controller/service.rs

-                        LogsBasedPartitionProcessorPlacementHints::from(&logs_controller))
-                    .await?;
+                    is_active_controller = self.is_active_controller(nodes_config.live_load(), &observed_cluster_state);
+                    trace!("Is active cluster controller leader: {is_active_controller}");


I would log on debug if the active status changes but not unconditionally on every cluster state update.

Or make a field of a span. But changing this field seems to lead to multiple entries as seen with the is_leader field of the partition processor.

tillrohrmann · 2024-11-15T18:08:08Z

crates/admin/src/cluster_controller/service.rs

+                            &mut logs_controller,
+                            &mut scheduler,
+                        ).await?;
+                        is_active_controller = true;


Should we also set is_active_controller to false if we lose leadership? Otherwise we might still act as the leader on subsequent loop iterations.

Side note, I am using naturally the term leadership when talking about active in active cluster controllers.

tillrohrmann · 2024-11-15T22:46:31Z

crates/admin/src/cluster_controller/scheduler.rs

+            if !is_active_controller {
+                break;
+            }


I think this works and I don't want to ask to do it differently now. What I am wondering, though, is whether making the leader state more explicit would help with cleaner code. If, for example, we would only create the Scheduler and LogsController when we are the leader this wouldn't be necessary. This would then also establish a clear lifecycle (e.g. when becoming the leader then we would take the current PartitionTable, LogsConfiguration and initialize the controllers) and makes clear which are the actions that a leader needs to do and what every follower. The downside would be if the init step when becoming the leader is expensive and there are many quick leader changes. In this case, the current way would probably behave more graceful.

I think I need to withdraw the correctness point. W/o the lower part, we wouldn't update the internal hold scheduling_plan. So if we now become leader, then we would start with a SchedulingPlan that is based on before this logs update. Differently said, with this if condition it would be equivalent to not call this method if we are not the leader.

My thinking was that if we become leader this will trigger a change in cluster state which will rebuild the scheduling plan based on latest known state.

I will definitely revise the code again taking that into account.

Maybe tracking the leader state in a state machine would be cleaner for reasoning and readability even if is a little bit slower to switch to leader mode?

muhamadazmy · 2024-11-17T13:57:40Z

crates/admin/src/cluster_controller/service.rs

+                _ = &mut shutdown => {
+                    self.health_status.update(AdminStatus::Unknown);
+                    return Ok(ClusterControllerStatus::Shutdown);
+                }


I don't believe changes in logs and/or partition table can trigger a reconfiguration of leadership of the CC, what do you think?

Yes, they shouldn't.

tillrohrmann

Thanks a lot for updating this PR @muhamadazmy. I think this approach looks a lot more robust since it makes it very clear what are the leader and what are the follower responsibilities. I left a comment how we could deduplicate some shared logic between the leader and follower state. Let me know what you think about it.

tillrohrmann · 2024-11-18T10:47:39Z

crates/admin/src/cluster_controller/service.rs

+    Passive,
+    Active,


Wondering whether we should call it leader and follower to align it with the leadership concept. That's something we already have somewhat established with the partition processor.

tillrohrmann · 2024-11-18T10:49:06Z

crates/admin/src/cluster_controller/service.rs

+            .sorted()
+            .next();
+
+        // A Cluster Controller is active if the node holds the smallest plain Node ID


Suggested change

// A Cluster Controller is active if the node holds the smallest plain Node ID

// A Cluster Controller is active if the node holds the smallest plain Node ID.

tillrohrmann · 2024-11-18T10:49:14Z

crates/admin/src/cluster_controller/service.rs

+            .next();
+
+        // A Cluster Controller is active if the node holds the smallest plain Node ID
+        // it's sometimes possible to have the smallest node id but not the highest generation


Suggested change

// it's sometimes possible to have the smallest node id but not the highest generation

// It's sometimes possible to have the smallest node id but not the highest generation

tillrohrmann · 2024-11-18T10:50:44Z

crates/admin/src/cluster_controller/service.rs

+                warn!(
+                    "Shuting down cluster controller because of a node generation id error {err}"
+                );
+                return ClusterControllerState::Shutdown;


Instead of modelling this as an explicit ClusterControllerState, would it also work to return it as a Shutdown error?

hmm, makes sense

tillrohrmann · 2024-11-18T10:57:59Z

crates/admin/src/cluster_controller/service.rs

+                return ClusterControllerState::Passive;
+            }
+
+            if let Err(err) = nodes_config.find_node_by_id(self.metadata.my_node_id()) {


I think that we can probably avoid this extra look-up the following way:

let maybe_active = nodes_config .get_admin_nodes() .filter_map(|node| { if self.observed_cluster_state .is_node_alive(node.current_generation) { Some(node.current_generation) } else { None } }) .sorted_by_key(|node_id| node_id.as_plain()) .next();

Then we get the generational node id.

tillrohrmann · 2024-11-18T10:59:42Z

crates/admin/src/cluster_controller/service.rs

+                _ = &mut shutdown => {
+                    self.health_status.update(AdminStatus::Unknown);
+                    return Ok(ClusterControllerStatus::Shutdown);
+                }


Yes, they shouldn't.

tillrohrmann · 2024-11-18T11:12:45Z

crates/admin/src/cluster_controller/service.rs

+            &self.bifrost,
+            &self.metadata_writer,
+            &self.metadata_store_client,
+        );


When initializing the Scheduler we probably need to call on_logs_update with the latest Logs from Metadata. This is currently missing in main as well.

Nit: The LogsController probably does not need to try to write an initial Logs configuration when being initialized. Instead it could issue a sync request and get the latest value from Metadata. Does not have to addressed here.

tillrohrmann · 2024-11-18T11:18:31Z

crates/admin/src/cluster_controller/service.rs

+        }
+    }
+
+    async fn passive(


passive and active share a bit of logic: The heartbeat_interval, cluste_state_watcher, command_rx, config_watcher and shutdown. Could we pull this logic up into the run method? That way we would share the common behavior and there is only a single place which needs to be kept in sync. The active method would then only run the parts that are specific to the leader state. What probably needs to happen for this is to make the ClusterControllerState an enum with fields:

enum ClusterControllerState { Active { scheduler: ..., logs_controller: ..., ... }, Passive, }

muhamadazmy · 2024-11-19T08:06:07Z

crates/admin/src/cluster_controller/service.rs

        self.health_status.update(AdminStatus::Ready);

        loop {
+            self.next_cluster_state(&mut state).await?;


I think this can be happen on cluster_state change only since (imho) it is the only way the leadership state can change, correct?

Yes, I think so.

tillrohrmann

Thanks a lot for updating the PR @mohammedazmy. I think this makes a lot of sense to me. I left a few minor comments regarding how we could split the service.rs into smaller modules and left a few questions. I think we can merge this PR today :-)

tillrohrmann · 2024-11-19T08:06:30Z

crates/admin/src/cluster_controller/service.rs

+        state: &mut ClusterControllerState<T>,
+    ) -> anyhow::Result<()> {
+        let nodes_config = self.metadata.nodes_config_ref();
+        let maybe_active = nodes_config


Suggested change

let maybe_active = nodes_config

let maybe_leader = nodes_config

tillrohrmann · 2024-11-19T08:06:50Z

crates/admin/src/cluster_controller/service.rs

-            &self.metadata_writer,
-            &self.metadata_store_client,
-        );
+        // A Cluster Controller is active if the node holds the smallest PlainNodeID


Suggested change

// A Cluster Controller is active if the node holds the smallest PlainNodeID

// A Cluster Controller is leader if the node holds the smallest PlainNodeID

tillrohrmann · 2024-11-19T08:07:54Z

crates/admin/src/cluster_controller/service.rs

-        let mut cluster_state_watcher = self.cluster_state_refresher.cluster_state_watcher();
+        let is_leader = match maybe_active {
+            None => true,
+            Some(leader) => leader == self.metadata.my_node_id().as_plain(),


Why using the plain node id and not the generational one?

GenerationalNodeId has derive Ord implementation. It means Ids with smaller generation id will show up first after sorting.

I did it then like this in case there is the same node is somehow still alive with 2 different generation. But now I am wondering if this can actually happen.

tillrohrmann · 2024-11-19T08:09:00Z

crates/admin/src/cluster_controller/service.rs


-        let mut scheduler = Scheduler::init(
-            configuration,
+    async fn create_leader<'a>(&self) -> anyhow::Result<Leader<T>> {


For what is the lifetime 'a used?

Ah sorry, leftover from some refactoring

tillrohrmann · 2024-11-19T08:10:50Z

crates/admin/src/cluster_controller/service.rs

                }
                Some(cmd) = self.command_rx.recv() => {
+                    // it is still safe to handle cluster commands as a passive CC


Suggested change

// it is still safe to handle cluster commands as a passive CC

// it is still safe to handle cluster commands as a follower

tillrohrmann · 2024-11-19T08:13:24Z

crates/admin/src/cluster_controller/service.rs

-                    return Ok(());
+                    anyhow::bail!(ShutdownError);


Why did you introduce this change from Ok(()) to anyhow::bail!?

My fault. This part was moved to another function then back to run() without restoring the original behaviour.

tillrohrmann · 2024-11-19T08:26:37Z

crates/admin/src/cluster_controller/service.rs

+enum ClusterControllerState<T> {
+    Follower,
+    Leader(Leader<T>),
+}
+
+impl<T> ClusterControllerState<T>
+where
+    T: TransportConnect,
+{
+    async fn run(&mut self) -> anyhow::Result<()> {
+        match self {
+            Self::Follower => {
+                futures::future::pending::<()>().await;
+                Ok(())
+            }
+            Self::Leader(leader) => leader.run().await,
+        }
+    }
+
+    async fn on_observed_cluster_state(
+        &mut self,
+        observed_cluster_state: &ObservedClusterState,
+    ) -> anyhow::Result<()> {
+        match self {
+            Self::Follower => Ok(()),
+            Self::Leader(leader) => {
+                leader
+                    .on_observed_cluster_state(observed_cluster_state)
+                    .await
+            }
+        }
+    }
+
+    fn reconfigure(&mut self, configuration: &Configuration) {
+        match self {
+            Self::Follower => {}
+            Self::Leader(leader) => leader.reconfigure(configuration),
+        }
+    }
+}
+
+struct Leader<T> {


Maybe move those types to a separate module to make the service.rs a bit smaller?

tillrohrmann · 2024-11-19T08:43:41Z

crates/admin/src/cluster_controller/service.rs

+    async fn create_leader<'a>(&self) -> anyhow::Result<Leader<T>> {
+        let configuration = self.configuration.pinned();
+
+        let scheduler = Scheduler::init(


To make changing leaderships properly work we probably need to initialize the Scheduler with the latest logs configuration. Otherwise we might miss it and therefore won't take any actions because we haven't seen the latest logs configuration which controls which partitions needs to be scheduled. This bug already contained in main.

crates/admin/src/cluster_controller/service.rs

tillrohrmann

Thanks for updating the PR @muhamadazmy. The changes look really nice :-) I left two minor comments. +1 for merging then.

crates/admin/src/cluster_controller/service.rs

tillrohrmann · 2024-11-19T13:41:54Z

crates/admin/src/cluster_controller/service.rs

+    pub(crate) task_center: TaskCenter,
+    pub(crate) metadata: Metadata,
+    pub(crate) networking: Networking<T>,
+    pub(crate) bifrost: Bifrost,
+    pub(crate) cluster_state_refresher: ClusterStateRefresher<T>,
+    pub(crate) configuration: Live<Configuration>,
+    pub(crate) metadata_writer: MetadataWriter,
+    pub(crate) metadata_store_client: MetadataStoreClient,


If cluster_controller_state were a module of service, then I believe you don't have to make these fields pub(crate).

tillrohrmann · 2024-11-19T13:44:35Z

crates/admin/src/cluster_controller/cluster_controller_state.rs

+        // mark both logs watcher and partition table watcher as changed to
+        // force updating the scheduler and logs_controller
+        leader.logs_watcher.mark_changed();
+        leader.partition_table_watcher.mark_changed();


That's a nice solution to the problem :-)

Fixes restatedev#2238

This was referenced Nov 8, 2024

Remove Attach rpc request #2250

Merged

A reusable MessageStream type alias #cleanup #2249

Merged

tillrohrmann self-requested a review November 9, 2024 15:25

muhamadazmy force-pushed the pr2252 branch 6 times, most recently from 0369a6c to af478e8 Compare November 14, 2024 14:37

muhamadazmy requested a review from AhmedSoliman November 14, 2024 14:44

muhamadazmy force-pushed the pr2252 branch from af478e8 to 0a1b99b Compare November 15, 2024 10:04

muhamadazmy commented Nov 15, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from 0a1b99b to 9f3038e Compare November 15, 2024 10:15

tillrohrmann reviewed Nov 15, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from 9f3038e to af2adcc Compare November 15, 2024 16:18

muhamadazmy requested a review from tillrohrmann November 15, 2024 16:18

muhamadazmy force-pushed the pr2252 branch from af2adcc to 304883d Compare November 15, 2024 16:24

tillrohrmann reviewed Nov 15, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from 304883d to 4e0ee5f Compare November 17, 2024 13:21

muhamadazmy requested a review from tillrohrmann November 17, 2024 13:24

muhamadazmy force-pushed the pr2252 branch from 4e0ee5f to 6d71f89 Compare November 17, 2024 13:56

muhamadazmy commented Nov 17, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch 3 times, most recently from 9a829f3 to aae0fea Compare November 18, 2024 07:43

muhamadazmy mentioned this pull request Nov 18, 2024

Cluster state types #2311

Closed

tillrohrmann reviewed Nov 18, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from aae0fea to 36191d2 Compare November 18, 2024 18:29

muhamadazmy requested a review from tillrohrmann November 18, 2024 18:31

muhamadazmy commented Nov 19, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from 36191d2 to 699fa87 Compare November 19, 2024 08:44

tillrohrmann reviewed Nov 19, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch 3 times, most recently from 1116010 to aedfddb Compare November 19, 2024 10:48

tillrohrmann approved these changes Nov 19, 2024

View reviewed changes

muhamadazmy force-pushed the pr2252 branch from aedfddb to c7aaa31 Compare November 19, 2024 17:27

muhamadazmy mentioned this pull request Nov 19, 2024

Handle stale metadata version when starting a CC leader #2325

Merged

Soft-leader election mechanism for cc based on observed cluster state

0873428

Fixes restatedev#2238

muhamadazmy force-pushed the pr2252 branch from c7aaa31 to 0873428 Compare November 19, 2024 18:18

muhamadazmy merged commit 0e00527 into restatedev:main Nov 20, 2024
18 checks passed

		// if nodes changes we need to try again in
		// case our leadership state has changed

	Ok(_) = partition_table_watcher.changed()=> {
	Ok(_) = partition_table_watcher.changed() => {

	if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) && !is_active_controller{
	if self.is_active_controller(nodes_config.live_load(), &observed_cluster_state) && !is_active_controller {

	trace!("Acting like a cluster controller");
	trace!("Acting as the active cluster controller");

	// A Cluster Controller is active if the node holds the smallest plain Node ID
	// A Cluster Controller is active if the node holds the smallest plain Node ID.

	// it's sometimes possible to have the smallest node id but not the highest generation
	// It's sometimes possible to have the smallest node id but not the highest generation

	let maybe_active = nodes_config
	let maybe_leader = nodes_config

	// A Cluster Controller is active if the node holds the smallest PlainNodeID
	// A Cluster Controller is leader if the node holds the smallest PlainNodeID

	// it is still safe to handle cluster commands as a passive CC
	// it is still safe to handle cluster commands as a follower

Soft-leader election mechanism for cc based on observed cluster state #2252

Soft-leader election mechanism for cc based on observed cluster state #2252

Conversation

muhamadazmy commented Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

tillrohrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhamadazmy Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhamadazmy commented Nov 15, 2024

muhamadazmy commented Nov 15, 2024

tillrohrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tillrohrmann Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tillrohrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tillrohrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tillrohrmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhamadazmy commented Nov 8, 2024 •

edited

Loading

muhamadazmy Nov 15, 2024 •

edited

Loading

tillrohrmann Nov 16, 2024 •

edited

Loading