make citizenlab_updater EXCHANGE ... ON CLUSTER

[aagbsn]

Similar to #185, the EXCHANGE here doesn't run ON CLUSTER, so the updates don't propagate to the rest of the nodes and could leave them pointing at a truncated table.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.81%. Comparing base (efb0f1d) to head (6a8d9ce).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #186      +/-   ##
==========================================
- Coverage   83.65%   82.81%   -0.85%     
==========================================
  Files          89       90       +1     
  Lines        5917     6022     +105     
==========================================
+ Hits         4950     4987      +37     
- Misses        967     1035      +68     

Flag	Coverage Δ
oonidata	77.87% <ø> (ø)
oonipipeline	85.15% <ø> (-1.32%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hellais]

This needs to be tested, but I believe that if we convert these tables to replicated tables, we will have to update the TRUNCATE citizenlab_flip to execute with alter_sync otherwise the INSERT operation will start running on the replicas before the TRUNCATE merge has completed running, leading to potentially duplicated data.

Given that these tables are very small, I'm also not sure we necessarily need a sharding key for them which probably simplifies things by not having a different ZK key. I saw a bug reported on the CH issue tracker mentioning that when that is the case there could be problems with TRUNCATE and EXCHANGE operations not converging: ClickHouse/ClickHouse#60489.

[aagbsn]

This needs to be tested, but I believe that if we convert these tables to replicated tables, we will have to update the TRUNCATE citizenlab_flip to execute with alter_sync otherwise the INSERT operation will start running on the replicas before the TRUNCATE merge has completed running, leading to potentially duplicated data.

Given that these tables are very small, I'm also not sure we necessarily need a sharding key for them which probably simplifies things by not having a different ZK key. I saw a bug reported on the CH issue tracker mentioning that when that is the case there could be problems with TRUNCATE and EXCHANGE operations not converging: ClickHouse/ClickHouse#60489.

Thanks for flagging this. From what I understand the TRUNCATE can be made to operate synchronously, but there's another problem which is that if a replica drops offline for a long enough time, the EXCHANGE is not applied, and that node would get out of sync with the rest of the replicas.

EXCHANGE/RENAME/CREATE via ON CLUSTER: this is a completely different, weaker mechanism. ON CLUSTER doesn't touch the table's own replication log at all — it writes a one-off task into a shared DDL queue at /clickhouse/task_queue/ddl in ZooKeeper. Every node in the cluster config independently polls that queue and executes the statement locally, then marks itself done. If a node happens to be offline when the task is queued, it's true that if a node is down, the task will time out and the initiating node returns an error, but the other nodes still complete the task when they come back online — so a node that's briefly offline and later reconnects with the same identity generally will catch up.

But that queue is not kept forever. It has a retention/cleanup window configured via task_max_lifetime, alongside a cleanup_delay_period that controls how often expired tasks are purged. If a replica is down longer than that window, the entry can be cleaned up before it ever reconnects — it just silently never sees that EXCHANGE.

There are two ways to mitigate this, one, is to swap the database engine to use ENGINE = Replicated, and the other is to ditch using an EXCHANGE atomic swap entirely and accept the empty-table window during the periodic update

[aagbsn]

There is actually another way to update the data without using the EXCHANGE .. ON CLUSTER semantics which is to use REPLACE PARTITION from a temporary table instead, which avoids potential issues with replicas that are down

[aagbsn]

added clustered clickhouse harness to test exchange partition replicates correctly

[hellais]

I think in reality we would want all replicas to converge before considering the task completed. If they don't converge then we ought to be showing an error, rather than silently swallowing it or hoping for the best.

[aagbsn]

this task runs frequently and only skips a replica that is actually offline; when it comes online it converges. It's only to prevent this task from blocking if a replica has an outage due to hardware failure or maintenance window

[hellais]

Our current server version is 24.8.6. Why are we using this specific version in here?

[aagbsn]

this is pinned to production version now

[aagbsn]

replaced by #190