Browse Source

Fix history out of bound access when replica is just created: make asan happy.
f21f17db000df2b0118361666d537ed5b213a7d4

osidorkin 8 months ago
parent
commit
3dc640b139

+ 16 - 7
yt/yt/client/chaos_client/replication_card.cpp

@@ -266,11 +266,20 @@ TReplicaInfo* TReplicationCard::GetReplicaOrThrow(TReplicaId replicaId, TReplica
 
 ////////////////////////////////////////////////////////////////////////////////
 
-bool IsReplicaSync(ETableReplicaMode mode, const TReplicaHistoryItem& lastReplicaHistoryItem)
+bool IsReplicaSync(ETableReplicaMode mode, const std::vector<TReplicaHistoryItem>& replicaHistory)
 {
     // Check actual replica state to avoid merging transition states (e.g. AsyncToSync -> SyncToAsync)
-    return mode == ETableReplicaMode::Sync ||
-        (mode == ETableReplicaMode::SyncToAsync && lastReplicaHistoryItem.IsSync());
+    if (mode == ETableReplicaMode::Sync) {
+        return true;
+    }
+
+    if (mode != ETableReplicaMode::SyncToAsync) {
+        return false;
+    }
+
+    // Replica in transient state MUST have previous non-transient state
+    YT_VERIFY(!replicaHistory.empty());
+    return replicaHistory.back().IsSync();
 }
 
 bool IsReplicaAsync(ETableReplicaMode mode)
@@ -291,9 +300,9 @@ bool IsReplicaDisabled(ETableReplicaState state)
 bool IsReplicaReallySync(
     ETableReplicaMode mode,
     ETableReplicaState state,
-    const TReplicaHistoryItem& lastReplicaHistoryItem)
+    const std::vector<TReplicaHistoryItem>& replicaHistory)
 {
-    return IsReplicaSync(mode, lastReplicaHistoryItem) && IsReplicaEnabled(state);
+    return IsReplicaSync(mode, replicaHistory) && IsReplicaEnabled(state);
 }
 
 ETableReplicaMode GetTargetReplicaMode(ETableReplicaMode mode)
@@ -854,7 +863,7 @@ THashMap<TReplicaId, TDuration> ComputeReplicasLag(const THashMap<TReplicaId, TR
 {
     TReplicationProgress syncProgress;
     for (const auto& [replicaId, replicaInfo] : replicas) {
-        if (IsReplicaReallySync(replicaInfo.Mode, replicaInfo.State, replicaInfo.History.back())) {
+        if (IsReplicaReallySync(replicaInfo.Mode, replicaInfo.State, replicaInfo.History)) {
             if (syncProgress.Segments.empty()) {
                 syncProgress = replicaInfo.ReplicationProgress;
             } else {
@@ -865,7 +874,7 @@ THashMap<TReplicaId, TDuration> ComputeReplicasLag(const THashMap<TReplicaId, TR
 
     THashMap<TReplicaId, TDuration> result;
     for (const auto& [replicaId, replicaInfo] : replicas) {
-        if (IsReplicaReallySync(replicaInfo.Mode, replicaInfo.State, replicaInfo.History.back())) {
+        if (IsReplicaReallySync(replicaInfo.Mode, replicaInfo.State, replicaInfo.History)) {
             result.emplace(replicaId, TDuration::Zero());
         } else {
             result.emplace(

+ 1 - 1
yt/yt/client/chaos_client/replication_card.h

@@ -131,7 +131,7 @@ bool IsReplicaDisabled(NTabletClient::ETableReplicaState state);
 bool IsReplicaReallySync(
     NTabletClient::ETableReplicaMode mode,
     NTabletClient::ETableReplicaState state,
-    const TReplicaHistoryItem& lastReplicaHistoryItem);
+    const std::vector<TReplicaHistoryItem>& replicaHistory);
 NTabletClient::ETableReplicaMode GetTargetReplicaMode(NTabletClient::ETableReplicaMode mode);
 NTabletClient::ETableReplicaState GetTargetReplicaState(NTabletClient::ETableReplicaState state);
 

+ 100 - 0
yt/yt/client/unittests/replication_card_ut.cpp

@@ -5,6 +5,8 @@
 namespace NYT::NChaosClient {
 namespace {
 
+using namespace NTabletClient;
+
 ////////////////////////////////////////////////////////////////////////////////
 
 class TReplicationCardFetchOptionsTest
@@ -94,5 +96,103 @@ INSTANTIATE_TEST_SUITE_P(
 
 ////////////////////////////////////////////////////////////////////////////////
 
+class TReplicationCardIsReplicaReallySyncTest
+    : public ::testing::Test
+    , public ::testing::WithParamInterface<std::tuple<
+        ETableReplicaMode,
+        ETableReplicaState,
+        const std::vector<TReplicaHistoryItem>,
+        bool>>
+{ };
+
+TEST_P(TReplicationCardIsReplicaReallySyncTest, IsReplicaReallySync)
+{
+    const auto& params = GetParam();
+    auto mode = std::get<0>(params);
+    auto state = std::get<1>(params);
+    const auto& history = std::get<2>(params);
+    auto expected = std::get<3>(params);
+
+    auto actual = IsReplicaReallySync(mode, state, history);
+
+    EXPECT_EQ(actual, expected)
+        << "progress: " << ToString(mode) << std::endl
+        << "update: " << ToString(state) << std::endl
+        << "history: " << ToString(history) << std::endl
+        << "actual: " << actual << std::endl;
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    TReplicationCardIsReplicaReallySyncTest,
+    TReplicationCardIsReplicaReallySyncTest,
+    ::testing::Values(
+        std::tuple(
+            ETableReplicaMode::Sync,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>(),
+            true),
+        std::tuple(
+            ETableReplicaMode::SyncToAsync,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>{
+                TReplicaHistoryItem{
+                    .Era = 0,
+                    .Timestamp = 0,
+                    .Mode = ETableReplicaMode::Sync,
+                    .State = ETableReplicaState::Enabled,
+                }
+            },
+            true),
+        std::tuple(
+            ETableReplicaMode::SyncToAsync,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>{
+                TReplicaHistoryItem{
+                    .Era = 0,
+                    .Timestamp = 0,
+                    .Mode = ETableReplicaMode::Sync,
+                    .State = ETableReplicaState::Disabled,
+                },
+                TReplicaHistoryItem{
+                    .Era = 1,
+                    .Timestamp = 1,
+                    .Mode = ETableReplicaMode::Sync,
+                    .State = ETableReplicaState::Enabled,
+                }
+            },
+            true),
+        std::tuple(
+            ETableReplicaMode::SyncToAsync,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>{
+                TReplicaHistoryItem{
+                    .Era = 0,
+                    .Timestamp = 0,
+                    .Mode = ETableReplicaMode::Async,
+                    .State = ETableReplicaState::Enabled,
+                }
+            },
+            false),
+        std::tuple(
+            ETableReplicaMode::SyncToAsync,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>{
+                TReplicaHistoryItem{
+                    .Era = 0,
+                    .Timestamp = 0,
+                    .Mode = ETableReplicaMode::Sync,
+                    .State = ETableReplicaState::Disabled,
+                }
+            },
+            false),
+        std::tuple(
+            ETableReplicaMode::Async,
+            ETableReplicaState::Enabled,
+            std::vector<TReplicaHistoryItem>(),
+            false)
+));
+
+////////////////////////////////////////////////////////////////////////////////
+
 } // namespace
 } // namespace NYT::NChaosClient