Intermediate changes

1 files changed, 12 insertions, 15 deletions

diff --git a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
index 79054ef46d..d83e9c3f80 100644
--- a/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
+++ b/contrib/clickhouse/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp
@@ -79,11 +79,10 @@ void ReplicatedMergeTreeRestartingThread::run()
 
     if (first_time)
     {
-        if (storage.is_readonly)
-        {
+        if (storage.is_readonly && !std::exchange(storage.is_readonly_metric_set, true))
             /// We failed to start replication, table is still readonly, so we should increment the metric. See also setNotReadonly().
             CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
-        }
+
         /// It does not matter if replication is actually started or not, just notify after the first attempt.
         storage.startup_event.set();
         first_time = false;
@@ -360,21 +359,19 @@ void ReplicatedMergeTreeRestartingThread::setReadonly(bool on_shutdown)
         return;
 
     if (became_readonly)
-        CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
-
-    /// Replica was already readonly, but we should decrement the metric, because we are detaching/dropping table.
-    /// if first pass wasn't done we don't have to decrement because it wasn't incremented in the first place
-    /// the task should be deactivated if it's full shutdown so no race is present
-    if (!first_time && on_shutdown)
     {
-        CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(!storage.is_readonly_metric_set);
+        storage.is_readonly_metric_set = true;
+        CurrentMetrics::add(CurrentMetrics::ReadonlyReplica);
+        return;
     }
 
-    if (storage.since_metadata_err_incr_readonly_metric)
+    /// Replica was already readonly, but we should decrement the metric if it was set because we are detaching/dropping table.
+    /// the task should be deactivated if it's full shutdown so no race is present
+    if (on_shutdown && std::exchange(storage.is_readonly_metric_set, false))
     {
         CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
     }
 }
 
@@ -384,10 +381,10 @@ void ReplicatedMergeTreeRestartingThread::setNotReadonly()
     /// is_readonly is true on startup, but ReadonlyReplica metric is not incremented,
     /// because we don't want to change this metric if replication is started successfully.
     /// So we should not decrement it when replica stopped being readonly on startup.
-    if (storage.is_readonly.compare_exchange_strong(old_val, false) && !first_time)
+    if (storage.is_readonly.compare_exchange_strong(old_val, false) && std::exchange(storage.is_readonly_metric_set, false))
     {
         CurrentMetrics::sub(CurrentMetrics::ReadonlyReplica);
-        assert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
+        chassert(CurrentMetrics::get(CurrentMetrics::ReadonlyReplica) >= 0);
     }
 }