aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorosidorkin <osidorkin@yandex-team.com>2024-06-18 17:51:35 +0300
committerosidorkin <osidorkin@yandex-team.com>2024-06-18 18:11:03 +0300
commit3dc640b139e0175239c26d2bf9013c90e106debe (patch)
tree1d561f16d9cd19e622a39cc87d8a0f1b8e68a1c2
parent33496e3c821cca6c048b5619de2f46988e1aad4c (diff)
downloadydb-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.cpp23
-rw-r--r--yt/yt/client/chaos_client/replication_card.h2
-rw-r--r--yt/yt/client/unittests/replication_card_ut.cpp100
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