Fix tests and add new

Author: kuznetsss

src/cluster/ClioNode.cpp

@@ -49,6 +49,10 @@ struct Fields {
 ClioNode
 ClioNode::from(ClioNode::UUID uuid, etl::WriterStateInterface const& writerState)
 {
+    // Determine the database role based on writer state priority:
+    // 1. ReadOnly takes precedence (configured mode)
+    // 2. Fallback mode indicates cluster-wide fallback mechanism is active
+    // 3. Otherwise, Writer or NotWriter based on current writing state
     auto const dbRole = [&writerState]() {
         if (writerState.isReadOnly()) {
             return ClioNode::DbRole::ReadOnly;

src/cluster/ClioNode.hpp

@@ -39,7 +39,19 @@ struct ClioNode {
      */
     static constexpr char const* kTIME_FORMAT = "%Y-%m-%dT%H:%M:%SZ";
 
-    /** @brief Database role */
+    /**
+     * @brief Database role of a node in the cluster.
+     *
+     * Roles are used to coordinate which node writes to the database:
+     * - ReadOnly: Node is configured to never write (strict read-only mode)
+     * - NotWriter: Node can write but is currently not the designated writer
+     * - Writer: Node is actively writing to the database
+     * - Fallback: Node is using the fallback writer decision mechanism
+     *
+     * When any node in the cluster is in Fallback mode, the entire cluster switches
+     * from the cluster communication mechanism to the slower but more reliable
+     * database-based conflict detection mechanism.
+     */
     enum class DbRole { ReadOnly = 0, NotWriter = 1, Writer = 2, Fallback = 3, MAX = 3 };
 
     using UUID = std::shared_ptr<boost::uuids::uuid>;

src/cluster/WriterDecider.cpp

@@ -51,13 +51,19 @@ WriterDecider::onNewState(ClioNode::cUUID selfId, std::shared_ptr<Backend::Clust
         [writerState = writerState_->clone(),
          selfId = std::move(selfId),
          clusterData = clusterData->value()](auto&&) mutable {
+            // Find this node's data in the cluster state
             auto const selfData =
                 std::ranges::find_if(clusterData, [&selfId](ClioNode const& node) { return node.uuid == selfId; });
             ASSERT(selfData != clusterData.end(), "Self data should always be in the cluster data");
+
+            // ReadOnly nodes never participate in writer decisions
+            // Fallback nodes have already switched to fallback mechanism
             if (selfData->dbRole == ClioNode::DbRole::ReadOnly or selfData->dbRole == ClioNode::DbRole::Fallback) {
                 return;
             }
 
+            // If any node in the cluster is in Fallback mode, the entire cluster must switch
+            // to the fallback writer decision mechanism for consistency
             if (std::ranges::any_of(clusterData, [](ClioNode const& node) {
                     return node.dbRole == ClioNode::DbRole::Fallback;
                 })) {

src/etl/ETLService.cpp

@@ -395,6 +395,8 @@ ETLService::startMonitor(uint32_t seq)
 
     monitorDbStalledSubscription_ = monitor_->subscribeToDbStalled([this]() {
         LOG(log_.warn()) << "ETLService received DbStalled signal from Monitor";
+        // Database stall detected - no writer has been active for 10 seconds
+        // This triggers the fallback mechanism and attempts to become the writer
         if (not state_->isStrictReadonly and not state_->isWriting)
             state_->writeCommandSignal(SystemState::WriteCommand::StartWriting);
         state_->isWriterDecidingFallback = true;
@@ -435,3 +437,4 @@ ETLService::giveUpWriter()
 }
 
 }  // namespace etl
+

src/etl/SystemState.hpp

@@ -113,11 +113,24 @@ struct SystemState {
         "Whether clio detected a corruption that needs manual attention"
     );
 
+    /**
+     * @brief Whether the cluster is using the fallback writer decision mechanism.
+     *
+     * The fallback mechanism is triggered when:
+     * - The database stalls for 10 seconds (detected by Monitor), indicating no active writer
+     * - A write conflict is detected, indicating multiple nodes attempting to write simultaneously
+     *
+     * When fallback mode is active, the cluster stops using the cluster communication mechanism
+     * (TTL-based role announcements) and relies on the slower but more reliable database-based
+     * conflict detection. This flag propagates across the cluster - if any node enters fallback
+     * mode, all nodes in the cluster will switch to fallback mode.
+     */
     util::prometheus::Bool isWriterDecidingFallback = PrometheusService::boolMetric(
         "etl_writing_deciding_fallback",
         util::prometheus::Labels{},
-        "Whether clio detected a corruption that needs manual attention"
+        "Whether the cluster is using the fallback writer decision mechanism"
     );
 };
 
 }  // namespace etl
+

src/etl/WriterState.hpp

@@ -68,9 +68,22 @@ class WriterStateInterface {
     virtual void
     giveUpWriting() = 0;
 
+    /**
+     * @brief Check if the cluster is using the fallback writer decision mechanism.
+     *
+     * @return true if the cluster has switched to fallback mode, false otherwise
+     */
     [[nodiscard]] virtual bool
     isFallback() const = 0;
 
+    /**
+     * @brief Switch the cluster to the fallback writer decision mechanism.
+     *
+     * This method is called when the cluster needs to transition from the cluster
+     * communication mechanism to the slower but more reliable fallback mechanism.
+     * Once set, this flag propagates to all nodes in the cluster through the
+     * ClioNode DbRole::Fallback state.
+     */
     virtual void
     setWriterDecidingFallback() = 0;
 
@@ -124,9 +137,20 @@ class WriterState : public WriterStateInterface {
     void
     giveUpWriting() override;
 
+    /**
+     * @brief Switch the cluster to the fallback writer decision mechanism.
+     *
+     * Sets the isWriterDecidingFallback flag in the system state, which will be
+     * propagated to other nodes in the cluster through the ClioNode DbRole::Fallback state.
+     */
     void
     setWriterDecidingFallback() override;
 
+    /**
+     * @brief Check if the cluster is using the fallback writer decision mechanism.
+     *
+     * @return true if the cluster has switched to fallback mode, false otherwise
+     */
     bool
     isFallback() const override;
 
@@ -135,3 +159,5 @@ class WriterState : public WriterStateInterface {
 };
 
 }  // namespace etl
+
+

src/etl/impl/Loading.cpp

@@ -75,6 +75,8 @@ Loader::load(model::LedgerData const& data)
                              << "; took " << duration << "ms";
 
             if (not success) {
+                // Write conflict detected - another node wrote to the database
+                // This triggers the fallback mechanism and stops this node from writing
                 state_->writeCommandSignal(SystemState::WriteCommand::StopWriting);
                 state_->isWriterDecidingFallback = true;
                 LOG(log_.warn()) << "Another node wrote a ledger into the DB - we have a write conflict";
@@ -155,3 +157,4 @@ Loader::loadInitialLedger(model::LedgerData const& data)
 }
 
 }  // namespace etl::impl
+

tests/unit/cluster/BackendTests.cpp

@@ -175,6 +175,7 @@ TEST_F(ClusterBackendTest, FetchClioNodesDataReturnsDataWithOtherNodes)
         );
     EXPECT_CALL(*backend_, writeNodeMessage).Times(testing::AtLeast(1));
     EXPECT_CALL(writerStateRef, isReadOnly).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
+    EXPECT_CALL(writerStateRef, isFallback).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
     EXPECT_CALL(writerStateRef, isWriting).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
     EXPECT_CALL(callbackMock, Call)
         .Times(testing::AtLeast(1))
@@ -253,6 +254,7 @@ TEST_F(ClusterBackendTest, WriteNodeMessageWritesSelfDataWithRecentTimestampAndD
         .Times(testing::AtLeast(1))
         .WillRepeatedly(testing::Return(BackendInterface::ClioNodesDataFetchResult{}));
     EXPECT_CALL(writerStateRef, isReadOnly).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
+    EXPECT_CALL(writerStateRef, isFallback).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
     EXPECT_CALL(writerStateRef, isWriting).Times(testing::AtLeast(1)).WillRepeatedly(testing::Return(false));
     EXPECT_CALL(*backend_, writeNodeMessage)
         .Times(testing::AtLeast(1))

tests/unit/cluster/ClioNodeTests.cpp

@@ -111,7 +111,8 @@ INSTANTIATE_TEST_SUITE_P(
     testing::Values(
         ClioNodeDbRoleTestBundle{.testName = "ReadOnly", .role = ClioNode::DbRole::ReadOnly},
         ClioNodeDbRoleTestBundle{.testName = "NotWriter", .role = ClioNode::DbRole::NotWriter},
-        ClioNodeDbRoleTestBundle{.testName = "Writer", .role = ClioNode::DbRole::Writer}
+        ClioNodeDbRoleTestBundle{.testName = "Writer", .role = ClioNode::DbRole::Writer},
+        ClioNodeDbRoleTestBundle{.testName = "Fallback", .role = ClioNode::DbRole::Fallback}
     ),
     tests::util::kNAME_GENERATOR
 );
@@ -153,6 +154,7 @@ TEST_F(ClioNodeDbRoleTest, DeserializationMissingDbRole)
 struct ClioNodeFromTestBundle {
     std::string testName;
     bool readOnly;
+    bool fallback;
     bool writing;
     ClioNode::DbRole expectedRole;
 };
@@ -170,18 +172,28 @@ INSTANTIATE_TEST_SUITE_P(
         ClioNodeFromTestBundle{
             .testName = "ReadOnly",
             .readOnly = true,
+            .fallback = false,
             .writing = false,
             .expectedRole = ClioNode::DbRole::ReadOnly
         },
+        ClioNodeFromTestBundle{
+            .testName = "Fallback",
+            .readOnly = false,
+            .fallback = true,
+            .writing = false,
+            .expectedRole = ClioNode::DbRole::Fallback
+        },
         ClioNodeFromTestBundle{
             .testName = "NotWriterNotReadOnly",
             .readOnly = false,
+            .fallback = false,
             .writing = false,
             .expectedRole = ClioNode::DbRole::NotWriter
         },
         ClioNodeFromTestBundle{
             .testName = "Writer",
             .readOnly = false,
+            .fallback = false,
             .writing = true,
             .expectedRole = ClioNode::DbRole::Writer
         }
@@ -195,7 +207,10 @@ TEST_P(ClioNodeFromTest, FromWriterState)
 
     EXPECT_CALL(writerState, isReadOnly()).WillOnce(testing::Return(param.readOnly));
     if (not param.readOnly) {
-        EXPECT_CALL(writerState, isWriting()).WillOnce(testing::Return(param.writing));
+        EXPECT_CALL(writerState, isFallback()).WillOnce(testing::Return(param.fallback));
+        if (not param.fallback) {
+            EXPECT_CALL(writerState, isWriting()).WillOnce(testing::Return(param.writing));
+        }
     }
 
     auto const beforeTime = std::chrono::system_clock::now();
@@ -207,3 +222,7 @@ TEST_P(ClioNodeFromTest, FromWriterState)
     EXPECT_GE(node.updateTime, beforeTime);
     EXPECT_LE(node.updateTime, afterTime);
 }
+
+
+
+

tests/unit/cluster/WriterDeciderTests.cpp

@@ -37,7 +37,7 @@
 
 using namespace cluster;
 
-enum class ExpectedAction { StartWriting, GiveUpWriting, NoAction };
+enum class ExpectedAction { StartWriting, GiveUpWriting, NoAction, SetFallback };
 
 struct WriterDeciderTestParams {
     std::string testName;
@@ -97,6 +97,10 @@ TEST_P(WriterDeciderTest, WriterSelection)
             EXPECT_CALL(*clonedState, giveUpWriting());
             EXPECT_CALL(writerStateRef, clone()).WillOnce(testing::Return(testing::ByMove(std::move(clonedState))));
             break;
+        case ExpectedAction::SetFallback:
+            EXPECT_CALL(*clonedState, setWriterDecidingFallback());
+            EXPECT_CALL(writerStateRef, clone()).WillOnce(testing::Return(testing::ByMove(std::move(clonedState))));
+            break;
         case ExpectedAction::NoAction:
             if (not params.useEmptyClusterData) {
                 // For all-ReadOnly case, we still clone but don't call any action
@@ -107,19 +111,25 @@ TEST_P(WriterDeciderTest, WriterSelection)
     }
 
     std::shared_ptr<Backend::ClusterData> clusterData;
+    ClioNode::cUUID selfIdPtr;
 
     if (params.useEmptyClusterData) {
         clusterData = std::make_shared<Backend::ClusterData>(std::unexpected(std::string("Communication failed")));
+        selfIdPtr = std::make_shared<boost::uuids::uuid>(selfUuid);
     } else {
         std::vector<ClioNode> nodes;
         nodes.reserve(params.nodes.size());
         for (auto const& [uuidValue, role] : params.nodes) {
-            nodes.push_back(makeNode(makeUuid(uuidValue), role));
+            auto node = makeNode(makeUuid(uuidValue), role);
+            if (uuidValue == params.selfUuidValue) {
+                selfIdPtr = node.uuid;  // Use the same shared_ptr as in the node
+            }
+            nodes.push_back(std::move(node));
         }
         clusterData = std::make_shared<Backend::ClusterData>(std::move(nodes));
     }
 
-    decider.onNewState(std::make_shared<boost::uuids::uuid>(selfUuid), clusterData);
+    decider.onNewState(selfIdPtr, clusterData);
 
     ctx.join();
 }
@@ -220,6 +230,43 @@ INSTANTIATE_TEST_SUITE_P(
                  {0x03, ClioNode::DbRole::Writer},
                  {0x02, ClioNode::DbRole::ReadOnly}},
             .expectedAction = ExpectedAction::StartWriting
+        },
+        WriterDeciderTestParams{
+            .testName = "SelfIsFallbackNoActionTaken",
+            .selfUuidValue = 0x01,
+            .nodes = {{0x01, ClioNode::DbRole::Fallback}, {0x02, ClioNode::DbRole::Writer}},
+            .expectedAction = ExpectedAction::NoAction
+        },
+        WriterDeciderTestParams{
+            .testName = "OtherNodeIsFallbackSetsFallbackMode",
+            .selfUuidValue = 0x01,
+            .nodes = {{0x01, ClioNode::DbRole::Writer}, {0x02, ClioNode::DbRole::Fallback}},
+            .expectedAction = ExpectedAction::SetFallback
+        },
+        WriterDeciderTestParams{
+            .testName = "SelfIsReadOnlyOthersAreFallbackNoActionTaken",
+            .selfUuidValue = 0x01,
+            .nodes = {{0x01, ClioNode::DbRole::ReadOnly}, {0x02, ClioNode::DbRole::Fallback}},
+            .expectedAction = ExpectedAction::NoAction
+        },
+        WriterDeciderTestParams{
+            .testName = "MultipleFallbackNodesSelfNotFallbackSetsFallback",
+            .selfUuidValue = 0x03,
+            .nodes =
+                {{0x01, ClioNode::DbRole::Fallback},
+                 {0x02, ClioNode::DbRole::Fallback},
+                 {0x03, ClioNode::DbRole::Writer}},
+            .expectedAction = ExpectedAction::SetFallback
+        },
+        WriterDeciderTestParams{
+            .testName = "MixedRolesWithOneFallbackSetsFallback",
+            .selfUuidValue = 0x02,
+            .nodes =
+                {{0x01, ClioNode::DbRole::Writer},
+                 {0x02, ClioNode::DbRole::NotWriter},
+                 {0x03, ClioNode::DbRole::Fallback},
+                 {0x04, ClioNode::DbRole::Writer}},
+            .expectedAction = ExpectedAction::SetFallback
         }
     ),
     [](testing::TestParamInfo<WriterDeciderTestParams> const& info) { return info.param.testName; }