From 44ea4baf02034f3e252978b446c065680141147c Mon Sep 17 00:00:00 2001 From: Daniil Komarevtsev Date: Tue, 28 Jan 2025 13:40:00 +0000 Subject: [PATCH] Review fixes --- .../blockstore/libs/storage/protos/disk.proto | 7 +- .../libs/storage/volume/model/helpers.cpp | 101 +++++------------- .../libs/storage/volume/model/helpers.h | 7 -- .../volume/volume_actor_lagging_agents.cpp | 5 +- 4 files changed, 33 insertions(+), 87 deletions(-) diff --git a/cloud/blockstore/libs/storage/protos/disk.proto b/cloud/blockstore/libs/storage/protos/disk.proto index 85a0fb9f360..c1e3ea91467 100644 --- a/cloud/blockstore/libs/storage/protos/disk.proto +++ b/cloud/blockstore/libs/storage/protos/disk.proto @@ -436,16 +436,13 @@ message TLaggingAgent // Agent id. string AgentId = 1; - // Node that agent is running on. - uint32 NodeId = 2; - // Index of the mirror disk replica. // 0 - main devices // 1,2 - replica devices - uint32 ReplicaIndex = 3; + uint32 ReplicaIndex = 2; // A list of devices that belong to the agent. - repeated TLaggingDevice Devices = 4; + repeated TLaggingDevice Devices = 3; } //////////////////////////////////////////////////////////////////////////////// diff --git a/cloud/blockstore/libs/storage/volume/model/helpers.cpp b/cloud/blockstore/libs/storage/volume/model/helpers.cpp index e40a062468d..a1c277a8bb6 100644 --- a/cloud/blockstore/libs/storage/volume/model/helpers.cpp +++ b/cloud/blockstore/libs/storage/volume/model/helpers.cpp @@ -14,7 +14,6 @@ namespace { //////////////////////////////////////////////////////////////////////////////// using google::protobuf::RepeatedPtrField; -using TDeviceMatcher = std::function; struct TLaggingDeviceIndexCmp { @@ -98,10 +97,30 @@ std::optional FindDeviceLocation( return std::nullopt; } -[[nodiscard]] std::optional FindReplicaIndex( +} // namespace + +//////////////////////////////////////////////////////////////////////////////// + +const NProto::TDeviceConfig* FindDeviceConfig( const NProto::TVolumeMeta& meta, - const TDeviceMatcher& deviceMatcher) + TStringBuf deviceUUID) { + auto deviceLocation = FindDeviceLocation(meta, deviceUUID); + if (!deviceLocation) { + return nullptr; + } + return &GetDeviceConfig(meta, *deviceLocation); +} + +std::optional FindReplicaIndexByAgentId( + const NProto::TVolumeMeta& meta, + TStringBuf agentId) +{ + const auto deviceMatcher = [agentId](const NProto::TDeviceConfig& device) + { + return device.GetAgentId() == agentId; + }; + for (size_t i = 0; i <= meta.ReplicasSize(); i++) { const auto& devices = GetReplicaDevices(meta, i); if (AnyOf(devices, deviceMatcher)) { @@ -122,11 +141,16 @@ std::optional FindDeviceLocation( return std::nullopt; } -[[nodiscard]] TVector CollectLaggingDevices( +TVector CollectLaggingDevices( const NProto::TVolumeMeta& meta, ui32 replicaIndex, - const TDeviceMatcher& deviceMatcher) + TStringBuf agentId) { + const auto deviceMatcher = [agentId](const NProto::TDeviceConfig& device) + { + return device.GetAgentId() == agentId; + }; + TVector result; const auto replicaDevices = GetReplicaDevices(meta, replicaIndex); for (int i = 0; i < replicaDevices.size(); i++) { @@ -163,73 +187,6 @@ std::optional FindDeviceLocation( return result; } -} // namespace - -//////////////////////////////////////////////////////////////////////////////// - -const NProto::TDeviceConfig* FindDeviceConfig( - const NProto::TVolumeMeta& meta, - TStringBuf deviceUUID) -{ - auto deviceLocation = FindDeviceLocation(meta, deviceUUID); - if (!deviceLocation) { - return nullptr; - } - return &GetDeviceConfig(meta, *deviceLocation); -} - -std::optional FindReplicaIndexByAgentNodeId( - const NProto::TVolumeMeta& meta, - ui32 agentNodeId) -{ - const auto deviceMatcher = - [agentNodeId](const NProto::TDeviceConfig& device) - { - return device.GetNodeId() == agentNodeId; - }; - - return FindReplicaIndex(meta, deviceMatcher); -} - -std::optional FindReplicaIndexByAgentId( - const NProto::TVolumeMeta& meta, - TStringBuf agentId) -{ - const auto deviceMatcher = [agentId](const NProto::TDeviceConfig& device) - { - return device.GetAgentId() == agentId; - }; - - return FindReplicaIndex(meta, deviceMatcher); -} - -TVector CollectLaggingDevices( - const NProto::TVolumeMeta& meta, - ui32 replicaIndex, - ui32 agentNodeId) -{ - const auto deviceMatcher = - [agentNodeId](const NProto::TDeviceConfig& device) - { - return device.GetNodeId() == agentNodeId; - }; - - return CollectLaggingDevices(meta, replicaIndex, deviceMatcher); -} - -TVector CollectLaggingDevices( - const NProto::TVolumeMeta& meta, - ui32 replicaIndex, - TStringBuf agentId) -{ - const auto deviceMatcher = [agentId](const NProto::TDeviceConfig& device) - { - return device.GetAgentId() == agentId; - }; - - return CollectLaggingDevices(meta, replicaIndex, deviceMatcher); -} - bool HaveCommonRows( const TVector& laggingCandidates, const RepeatedPtrField& alreadyLagging) diff --git a/cloud/blockstore/libs/storage/volume/model/helpers.h b/cloud/blockstore/libs/storage/volume/model/helpers.h index 919093bcfa0..2409a8d5daf 100644 --- a/cloud/blockstore/libs/storage/volume/model/helpers.h +++ b/cloud/blockstore/libs/storage/volume/model/helpers.h @@ -11,17 +11,10 @@ namespace NCloud::NBlockStore::NStorage { const NProto::TVolumeMeta& meta, TStringBuf deviceUUID); -[[nodiscard]] std::optional FindReplicaIndexByAgentNodeId( - const NProto::TVolumeMeta& meta, - ui32 agentNodeId); [[nodiscard]] std::optional FindReplicaIndexByAgentId( const NProto::TVolumeMeta& meta, TStringBuf agentId); -[[nodiscard]] TVector CollectLaggingDevices( - const NProto::TVolumeMeta& meta, - ui32 replicaIndex, - ui32 agentNodeId); [[nodiscard]] TVector CollectLaggingDevices( const NProto::TVolumeMeta& meta, ui32 replicaIndex, diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_lagging_agents.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_lagging_agents.cpp index 32fafd97d0c..b75119860d4 100644 --- a/cloud/blockstore/libs/storage/volume/volume_actor_lagging_agents.cpp +++ b/cloud/blockstore/libs/storage/volume/volume_actor_lagging_agents.cpp @@ -130,14 +130,14 @@ void TVolumeActor::HandleDeviceTimeouted( } const auto timeoutedDeviceReplicaIndex = - FindReplicaIndexByAgentNodeId(meta, timeoutedDeviceConfig->GetNodeId()); + FindReplicaIndexByAgentId(meta, timeoutedDeviceConfig->GetAgentId()); Y_DEBUG_ABORT_UNLESS(timeoutedDeviceReplicaIndex); TVector timeoutedAgentDevices = CollectLaggingDevices( meta, *timeoutedDeviceReplicaIndex, - timeoutedDeviceConfig->GetNodeId()); + timeoutedDeviceConfig->GetAgentId()); Y_DEBUG_ABORT_UNLESS(!timeoutedAgentDevices.empty()); for (const auto& laggingAgent: meta.GetLaggingAgentsInfo().GetAgents()) { @@ -223,7 +223,6 @@ void TVolumeActor::HandleDeviceTimeouted( NProto::TLaggingAgent unavailableAgent; unavailableAgent.SetAgentId(timeoutedDeviceConfig->GetAgentId()); - unavailableAgent.SetNodeId(timeoutedDeviceConfig->GetNodeId()); unavailableAgent.SetReplicaIndex(*timeoutedDeviceReplicaIndex); unavailableAgent.MutableDevices()->Assign( std::make_move_iterator(timeoutedAgentDevices.begin()),