From 81d4eb3e880de8eab58c2029004d76a512370739 Mon Sep 17 00:00:00 2001 From: Andrei Rykov Date: Fri, 1 Mar 2024 09:08:01 +0100 Subject: [PATCH 1/2] YDB-1470 added database filter for time difference (#2217) --- .../run/kikimr_services_initializers.cpp | 6 +++-- ydb/core/health_check/health_check.cpp | 23 +++++++++++++++---- .../actors/interconnect/interconnect_common.h | 5 +++- .../interconnect/interconnect_tcp_session.cpp | 7 +++++- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp index 54c0c3b273c7..20e4624fd5d8 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp @@ -712,8 +712,10 @@ void TBasicServicesInitializer::InitializeServices(NActors::TActorSystemSetup* s data.Yellow ? NKikimrWhiteboard::EFlag::Yellow : data.Orange ? NKikimrWhiteboard::EFlag::Orange : data.Red ? NKikimrWhiteboard::EFlag::Red : NKikimrWhiteboard::EFlag())); - data.ActorSystem->Send(whiteboardId, new NNodeWhiteboard::TEvWhiteboard::TEvClockSkewUpdate( - data.PeerId, data.ClockSkew)); + if (data.ReportClockSkew) { + data.ActorSystem->Send(whiteboardId, new NNodeWhiteboard::TEvWhiteboard::TEvClockSkewUpdate( + data.PeerId, data.ClockSkew)); + } }; } diff --git a/ydb/core/health_check/health_check.cpp b/ydb/core/health_check/health_check.cpp index 07de22de1eaa..8c593dd83267 100644 --- a/ydb/core/health_check/health_check.cpp +++ b/ydb/core/health_check/health_check.cpp @@ -519,6 +519,20 @@ class TSelfCheckRequest : public TActorBootstrapped { return FilterDatabase && FilterDatabase != DomainPath; } + bool IsTimeDifferenceCheckNode(const TNodeId nodeId) const { + if (!IsSpecificDatabaseFilter()) { + return true; + } + + auto it = DatabaseState.find(FilterDatabase); + if (it == DatabaseState.end()) { + return false; + } + auto& computeNodeIds = it->second.ComputeNodeIds; + + return std::find(computeNodeIds.begin(), computeNodeIds.end(), nodeId) != computeNodeIds.end(); + } + void Bootstrap() { FilterDatabase = Request->Database; if (Request->Request.operation_params().has_operation_timeout()) { @@ -813,10 +827,10 @@ class TSelfCheckRequest : public TActorBootstrapped { ReplyAndPassAway(); } - bool IsStaticNode(const TEvInterconnect::TNodeInfo& nodeInfo) const { + bool IsStaticNode(const TNodeId nodeId) const { TAppData* appData = AppData(); if (appData->DynamicNameserviceConfig) { - return nodeInfo.NodeId <= AppData()->DynamicNameserviceConfig->MaxStaticNodeId; + return nodeId <= AppData()->DynamicNameserviceConfig->MaxStaticNodeId; } else { return true; } @@ -827,7 +841,7 @@ class TSelfCheckRequest : public TActorBootstrapped { NodesInfo = ev->Release(); for (const auto& ni : NodesInfo->Nodes) { MergedNodeInfo[ni.NodeId] = ∋ - if (IsStaticNode(ni) && needComputeFromStaticNodes) { + if (IsStaticNode(ni.NodeId) && needComputeFromStaticNodes) { DatabaseState[DomainPath].ComputeNodeIds.push_back(ni.NodeId); RequestComputeNode(ni.NodeId); } @@ -2077,7 +2091,8 @@ class TSelfCheckRequest : public TActorBootstrapped { TNodeId maxClockSkewPeerId = 0; TNodeId maxClockSkewNodeId = 0; for (auto& [nodeId, nodeSystemState] : MergedNodeSystemState) { - if (abs(nodeSystemState->GetMaxClockSkewWithPeerUs()) > maxClockSkewUs) { + if (IsTimeDifferenceCheckNode(nodeId) && IsTimeDifferenceCheckNode(nodeSystemState->GetMaxClockSkewPeerId()) + && abs(nodeSystemState->GetMaxClockSkewWithPeerUs()) > maxClockSkewUs) { maxClockSkewUs = abs(nodeSystemState->GetMaxClockSkewWithPeerUs()); maxClockSkewPeerId = nodeSystemState->GetMaxClockSkewPeerId(); maxClockSkewNodeId = nodeId; diff --git a/ydb/library/actors/interconnect/interconnect_common.h b/ydb/library/actors/interconnect/interconnect_common.h index 29f32256838b..fc23f70b5315 100644 --- a/ydb/library/actors/interconnect/interconnect_common.h +++ b/ydb/library/actors/interconnect/interconnect_common.h @@ -74,8 +74,10 @@ namespace NActors { bool Orange; bool Red; i64 ClockSkew; + bool ReportClockSkew; - TWhiteboardSessionStatus(TActorSystem* actorSystem, ui32 peerId, const TString& peer, bool connected, bool green, bool yellow, bool orange, bool red, i64 clockSkew) + TWhiteboardSessionStatus(TActorSystem* actorSystem, ui32 peerId, const TString& peer, bool connected, + bool green, bool yellow, bool orange, bool red, i64 clockSkew, bool reportClockSkew) : ActorSystem(actorSystem) , PeerId(peerId) , Peer(peer) @@ -85,6 +87,7 @@ namespace NActors { , Orange(orange) , Red(red) , ClockSkew(clockSkew) + , ReportClockSkew(reportClockSkew) {} }; diff --git a/ydb/library/actors/interconnect/interconnect_tcp_session.cpp b/ydb/library/actors/interconnect/interconnect_tcp_session.cpp index e1f2bc735130..a5ecb2052137 100644 --- a/ydb/library/actors/interconnect/interconnect_tcp_session.cpp +++ b/ydb/library/actors/interconnect/interconnect_tcp_session.cpp @@ -999,6 +999,10 @@ namespace NActors { } while (false); } + // we need track clockskew only if it's one tenant nodes connection + // they have one scope in this case + bool reportClockSkew = Proxy->Common->LocalScopeId.first != 0 && Proxy->Common->LocalScopeId == Params.PeerScopeId; + callback({TlsActivationContext->ExecutorThread.ActorSystem, Proxy->PeerNodeId, Proxy->Metrics->GetHumanFriendlyPeerHostName(), @@ -1007,7 +1011,8 @@ namespace NActors { flagState == EFlag::YELLOW, flagState == EFlag::ORANGE, flagState == EFlag::RED, - ReceiveContext->ClockSkew_us.load()}); + ReceiveContext->ClockSkew_us.load(), + reportClockSkew}); } if (connected) { From eabeacd0068e140a9ecdd413a338444aa33f1fb7 Mon Sep 17 00:00:00 2001 From: Andrei Rykov Date: Wed, 3 Jul 2024 07:50:02 +0200 Subject: [PATCH 2/2] move time difference issue under database level (#5859) --- ydb/core/health_check/health_check.cpp | 80 ++++++++++++---------- ydb/public/api/protos/ydb_monitoring.proto | 8 +++ 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/ydb/core/health_check/health_check.cpp b/ydb/core/health_check/health_check.cpp index 8c593dd83267..a52309771db8 100644 --- a/ydb/core/health_check/health_check.cpp +++ b/ydb/core/health_check/health_check.cpp @@ -235,6 +235,7 @@ class TSelfCheckRequest : public TActorBootstrapped { ui64 StorageQuota; ui64 StorageUsage; TMaybeServerlessComputeResourcesMode ServerlessComputeResourcesMode; + TNodeId MaxTimeDifferenceNodeId = 0; }; struct TSelfCheckResult { @@ -1265,7 +1266,7 @@ class TSelfCheckRequest : public TActorBootstrapped { } } - void FillComputeNodeStatus(TDatabaseState& databaseState,TNodeId nodeId, Ydb::Monitoring::ComputeNodeStatus& computeNodeStatus, TSelfCheckContext context) { + void FillComputeNodeStatus(TDatabaseState& databaseState, TNodeId nodeId, Ydb::Monitoring::ComputeNodeStatus& computeNodeStatus, TSelfCheckContext context) { FillNodeInfo(nodeId, context.Location.mutable_compute()->mutable_node()); TSelfCheckContext rrContext(&context, "NODE_UPTIME"); @@ -1303,6 +1304,34 @@ class TSelfCheckRequest : public TActorBootstrapped { } loadAverageStatus.set_overall(laContext.GetOverallStatus()); } + + if (nodeSystemState.HasMaxClockSkewPeerId()) { + TNodeId peerId = nodeSystemState.GetMaxClockSkewPeerId(); + long timeDifferenceUs = nodeSystemState.GetMaxClockSkewWithPeerUs(); + TDuration timeDifferenceDuration = TDuration::MicroSeconds(abs(timeDifferenceUs)); + Ydb::Monitoring::StatusFlag::Status status; + if (timeDifferenceDuration > MAX_CLOCKSKEW_ORANGE_ISSUE_TIME) { + status = Ydb::Monitoring::StatusFlag::ORANGE; + } else if (timeDifferenceDuration > MAX_CLOCKSKEW_YELLOW_ISSUE_TIME) { + status = Ydb::Monitoring::StatusFlag::YELLOW; + } else { + status = Ydb::Monitoring::StatusFlag::GREEN; + } + + computeNodeStatus.mutable_max_time_difference()->set_peer(ToString(peerId)); + computeNodeStatus.mutable_max_time_difference()->set_difference_ms(timeDifferenceDuration.MilliSeconds()); + computeNodeStatus.set_overall(status); + + if (DatabaseState.MaxTimeDifferenceNodeId == nodeId) { + TSelfCheckContext tdContext(&context, "NODES_TIME_DIFFERENCE"); + FillNodeInfo(peerId, tdContext.Location.mutable_compute()->mutable_peer()); + if (status == Ydb::Monitoring::StatusFlag::GREEN) { + tdContext.ReportStatus(status); + } else { + tdContext.ReportStatus(status, TStringBuilder() << "The nodes have a time difference of " << timeDifferenceDuration.MilliSeconds() << " ms", ETags::SyncState); + } + } + } } else { // context.ReportStatus(Ydb::Monitoring::StatusFlag::RED, // TStringBuilder() << "Compute node is not available", @@ -1334,12 +1363,24 @@ class TSelfCheckRequest : public TActorBootstrapped { if (systemStatus != Ydb::Monitoring::StatusFlag::GREEN && systemStatus != Ydb::Monitoring::StatusFlag::GREY) { context.ReportStatus(systemStatus, "Compute has issues with system tablets", ETags::ComputeState, {ETags::SystemTabletState}); } + long maxTimeDifferenceUs = 0; + for (TNodeId nodeId : *computeNodeIds) { + auto itNodeSystemState = MergedNodeSystemState.find(nodeId); + if (itNodeSystemState != MergedNodeSystemState.end()) { + if (std::count(computeNodeIds->begin(), computeNodeIds->end(), itNodeSystemState->second->GetMaxClockSkewPeerId()) > 0 + && abs(itNodeSystemState->second->GetMaxClockSkewWithPeerUs()) > maxTimeDifferenceUs) { + maxTimeDifferenceUs = abs(itNodeSystemState->second->GetMaxClockSkewWithPeerUs()); + databaseState.MaxTimeDifferenceNodeId = nodeId; + } + } + } for (TNodeId nodeId : *computeNodeIds) { auto& computeNode = *computeStatus.add_nodes(); - FillComputeNodeStatus(databaseState, nodeId, computeNode, {&context, "COMPUTE_NODE"}); + FillComputeNodeStatus(databaseState, nodeId, computeNode, {&context, "COMPUTE_NODE"}, maxClockSkewNodeId == nodeId); } context.ReportWithMaxChildStatus("Some nodes are restarting too often", ETags::ComputeState, {ETags::Uptime}); context.ReportWithMaxChildStatus("Compute is overloaded", ETags::ComputeState, {ETags::OverloadState}); + context.ReportWithMaxChildStatus("Database has time difference between nodes", ETags::ComputeState, {ETags::SyncState}); Ydb::Monitoring::StatusFlag::Status tabletsStatus = Ydb::Monitoring::StatusFlag::GREEN; computeNodeIds->push_back(0); // for tablets without node for (TNodeId nodeId : *computeNodeIds) { @@ -2086,40 +2127,6 @@ class TSelfCheckRequest : public TActorBootstrapped { const TDuration MAX_CLOCKSKEW_ORANGE_ISSUE_TIME = TDuration::MicroSeconds(25000); const TDuration MAX_CLOCKSKEW_YELLOW_ISSUE_TIME = TDuration::MicroSeconds(5000); - void FillNodesSyncStatus(TOverallStateContext& context) { - long maxClockSkewUs = 0; - TNodeId maxClockSkewPeerId = 0; - TNodeId maxClockSkewNodeId = 0; - for (auto& [nodeId, nodeSystemState] : MergedNodeSystemState) { - if (IsTimeDifferenceCheckNode(nodeId) && IsTimeDifferenceCheckNode(nodeSystemState->GetMaxClockSkewPeerId()) - && abs(nodeSystemState->GetMaxClockSkewWithPeerUs()) > maxClockSkewUs) { - maxClockSkewUs = abs(nodeSystemState->GetMaxClockSkewWithPeerUs()); - maxClockSkewPeerId = nodeSystemState->GetMaxClockSkewPeerId(); - maxClockSkewNodeId = nodeId; - } - } - if (!maxClockSkewNodeId) { - return; - } - - TSelfCheckResult syncContext; - syncContext.Type = "NODES_TIME_DIFFERENCE"; - FillNodeInfo(maxClockSkewNodeId, syncContext.Location.mutable_node()); - FillNodeInfo(maxClockSkewPeerId, syncContext.Location.mutable_peer()); - - TDuration maxClockSkewTime = TDuration::MicroSeconds(maxClockSkewUs); - if (maxClockSkewTime > MAX_CLOCKSKEW_ORANGE_ISSUE_TIME) { - syncContext.ReportStatus(Ydb::Monitoring::StatusFlag::ORANGE, TStringBuilder() << "The nodes have a time difference of " << maxClockSkewTime.MilliSeconds() << " ms", ETags::SyncState); - } else if (maxClockSkewTime > MAX_CLOCKSKEW_YELLOW_ISSUE_TIME) { - syncContext.ReportStatus(Ydb::Monitoring::StatusFlag::YELLOW, TStringBuilder() << "The nodes have a time difference of " << maxClockSkewTime.MilliSeconds() << " ms", ETags::SyncState); - } else { - syncContext.ReportStatus(Ydb::Monitoring::StatusFlag::GREEN); - } - - context.UpdateMaxStatus(syncContext.GetOverallStatus()); - context.AddIssues(syncContext.IssueRecords); - } - void FillResult(TOverallStateContext context) { if (IsSpecificDatabaseFilter()) { FillDatabaseResult(context, FilterDatabase, DatabaseState[FilterDatabase]); @@ -2128,7 +2135,6 @@ class TSelfCheckRequest : public TActorBootstrapped { FillDatabaseResult(context, path, state); } } - FillNodesSyncStatus(context); if (DatabaseState.empty()) { Ydb::Monitoring::DatabaseStatus& databaseStatus(*context.Result->add_database_status()); TSelfCheckResult tabletContext; diff --git a/ydb/public/api/protos/ydb_monitoring.proto b/ydb/public/api/protos/ydb_monitoring.proto index d58cd6c9988f..67b30f28c3fb 100644 --- a/ydb/public/api/protos/ydb_monitoring.proto +++ b/ydb/public/api/protos/ydb_monitoring.proto @@ -106,12 +106,19 @@ message LoadAverageStatus { uint32 cores = 3; } +message TimeDifferenceStatus { + StatusFlag.Status overall = 1; + int64 difference_ms = 2; + string peer = 3; +} + message ComputeNodeStatus { string id = 1; StatusFlag.Status overall = 2; repeated ComputeTabletStatus tablets = 3; repeated ThreadPoolStatus pools = 4; LoadAverageStatus load = 5; + TimeDifferenceStatus max_time_difference = 6; } message ComputeStatus { @@ -165,6 +172,7 @@ message LocationCompute { LocationNode node = 1; LocationComputePool pool = 2; LocationComputeTablet tablet = 3; + LocationNode peer = 4; } message LocationDatabase {