clickhouse: sync parts after system restore replica

This prevents a race condition in the tests (but also in prod) where Astacus exits claiming a restore has happened but instead there are still parts being synced. [DDB-922]
Aiven-Open · Apr 9, 2024 · 7db38ab · 7db38ab
1 parent a505a5b
commit 7db38ab
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 15 deletions.
diff --git a/astacus/coordinator/plugins/clickhouse/plugin.py b/astacus/coordinator/plugins/clickhouse/plugin.py
@@ -2,6 +2,7 @@
 Copyright (c) 2021 Aiven Ltd
 See LICENSE for details
 """
+
 from .config import (
     ClickHouseConfiguration,
     DiskConfiguration,
@@ -187,11 +188,6 @@ def get_restore_steps(self, *, context: OperationContext, req: RestoreRequest) -
                 attach_timeout=self.attach_timeout,
                 max_concurrent_attach_per_node=self.max_concurrent_attach_per_node,
             ),
-            SyncTableReplicasStep(
-                clients=clients,
-                sync_timeout=self.sync_tables_timeout,
-                max_concurrent_sync_per_node=self.max_concurrent_sync_per_node,
-            ),
             RestoreReplicaStep(
                 zookeeper_client=zookeeper_client,
                 clients=clients,
@@ -201,6 +197,11 @@ def get_restore_steps(self, *, context: OperationContext, req: RestoreRequest) -
                 restore_timeout=self.restore_replica_timeout,
                 max_concurrent_restore_per_node=self.max_concurrent_restore_replica_per_node,
             ),
+            SyncTableReplicasStep(
+                clients=clients,
+                sync_timeout=self.sync_tables_timeout,
+                max_concurrent_sync_per_node=self.max_concurrent_sync_per_node,
+            ),
             # Keeping this step last avoids access from non-admin users while we are still restoring
             RestoreAccessEntitiesStep(
                 zookeeper_client=zookeeper_client, access_entities_path=self.replicated_access_zookeeper_path

diff --git a/astacus/coordinator/plugins/clickhouse/steps.py b/astacus/coordinator/plugins/clickhouse/steps.py
@@ -2,6 +2,7 @@
 Copyright (c) 2021 Aiven Ltd
 See LICENSE for details
 """
+
 from __future__ import annotations
 
 from .client import ClickHouseClient, ClickHouseClientQueryError, escape_sql_identifier, escape_sql_string
@@ -758,8 +759,6 @@ class SyncTableReplicasStep(Step[None]):
 
     async def run_step(self, cluster: Cluster, context: StepsContext) -> None:
         manifest = context.get_result(ClickHouseManifestStep)
-        if manifest.version != ClickHouseBackupVersion.V1:
-            return
 
         def _sync_replicas(client: ClickHouseClient) -> Iterator[Awaitable[None]]:
             yield from (

diff --git a/tests/integration/coordinator/plugins/clickhouse/test_plugin.py b/tests/integration/coordinator/plugins/clickhouse/test_plugin.py
@@ -35,14 +35,6 @@
 import pytest
 import tempfile
 
-pytest.skip(
-    "These are flakey see tickets"
-    "https://aiven.atlassian.net/jira/software/c/projects/DDB/boards/210?selectedIssue=DDB-922"
-    "and https://aiven.atlassian.net/jira/software/c/projects/DDB/boards/210?selectedIssue=DDB-932",
-    allow_module_level=True,
-)
-
-
 pytestmark = [
     pytest.mark.clickhouse,
     pytest.mark.order("last"),