diff options
author | osidorkin <osidorkin@yandex-team.com> | 2024-06-18 17:51:35 +0300 |
---|---|---|
committer | osidorkin <osidorkin@yandex-team.com> | 2024-06-18 18:11:03 +0300 |
commit | 3dc640b139e0175239c26d2bf9013c90e106debe (patch) | |
tree | 1d561f16d9cd19e622a39cc87d8a0f1b8e68a1c2 | |
parent | 33496e3c821cca6c048b5619de2f46988e1aad4c (diff) | |
download | ydb-3dc640b139e0175239c26d2bf9013c90e106debe.tar.gz |
Fix history out of bound access when replica is just created: make asan happy.
f21f17db000df2b0118361666d537ed5b213a7d4
-rw-r--r-- | yt/yt/client/chaos_client/replication_card.cpp | 23 | ||||
-rw-r--r-- | yt/yt/client/chaos_client/replication_card.h | 2 | ||||
-rw-r--r-- | yt/yt/client/unittests/replication_card_ut.cpp | 100 |
3 files changed, 117 insertions, 8 deletions
diff --git a/yt/yt/client/chaos_client/replication_card.cpp b/yt/yt/client/chaos_client/replication_card.cpp index dcafa48f63..7a36a4e95c 100644 --- a/yt/yt/client/chaos_client/replication_card.cpp +++ b/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( diff --git a/yt/yt/client/chaos_client/replication_card.h b/yt/yt/client/chaos_client/replication_card.h index 9581f887f7..7462e160ee 100644 --- a/yt/yt/client/chaos_client/replication_card.h +++ b/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); diff --git a/yt/yt/client/unittests/replication_card_ut.cpp b/yt/yt/client/unittests/replication_card_ut.cpp index 97a5b32dc6..365e215b1d 100644 --- a/yt/yt/client/unittests/replication_card_ut.cpp +++ b/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 |