From 863cc65325bd58aca2cede7c358ee06c61483677 Mon Sep 17 00:00:00 2001 From: Kirill Pleshivtsev Date: Mon, 29 Jul 2024 11:50:59 +0300 Subject: [PATCH] Fix channel count overflow (#1645) * Fix channel count overflow * Fix review issues * Fix params mess * Making calculations simpler * Remove unused param --- .../libs/storage/core/volume_model.cpp | 70 ++++++++ .../libs/storage/core/volume_model.h | 6 + .../libs/storage/core/volume_model_ut.cpp | 166 ++++++++++++++++++ 3 files changed, 242 insertions(+) diff --git a/cloud/blockstore/libs/storage/core/volume_model.cpp b/cloud/blockstore/libs/storage/core/volume_model.cpp index e42b99ec03..c8533fadda 100644 --- a/cloud/blockstore/libs/storage/core/volume_model.cpp +++ b/cloud/blockstore/libs/storage/core/volume_model.cpp @@ -6,8 +6,10 @@ #include +#include #include #include +#include #include @@ -367,6 +369,15 @@ void SetExplicitChannelProfiles( ++c; } + ComputeChannelCountLimits( + 255 - c, + &mergedChannels, + &mixedChannels, + &freshChannels); + + Y_DEBUG_ABORT_UNLESS( + c + mergedChannels + mixedChannels + freshChannels <= 255); + while (mergedChannels > 0) { AddOrModifyChannel( poolKinds.Merged, @@ -808,4 +819,63 @@ ui64 ComputeMaxBlocks( return MaxPartitionBlocksCount; } +//////////////////////////////////////////////////////////////////////////////// + +void ComputeChannelCountLimits( + int freeChannelCount, + int* wantToAddMerged, + int* wantToAddMixed, + int* wantToAddFresh) +{ + int* wantToAddByKind[] = { + wantToAddMerged, + wantToAddMixed, + wantToAddFresh, + }; + + if (!freeChannelCount) { + // There are no free channels. + for (auto& wantToAdd: wantToAddByKind) { + *wantToAdd = 0; + } + return; + } + + auto totalWantToAddCalculator = [&]() -> int + { + return Accumulate( + wantToAddByKind, + int{}, + [](int acc, const int* wantToAdd) { return acc + *wantToAdd; }); + }; + + auto totalWantToAdd = totalWantToAddCalculator(); + if (totalWantToAdd <= freeChannelCount) { + // There are enough free channels to satisfy everyone. + return; + } + + // Distribute free channels among all channel kinds. + for (auto& wantToAdd: wantToAddByKind) { + double trimmedWantToAdd = + static_cast(*wantToAdd) * freeChannelCount / totalWantToAdd; + if (trimmedWantToAdd > 0.0 && trimmedWantToAdd < 1.0) { + *wantToAdd = 1; + } else { + *wantToAdd = std::round(trimmedWantToAdd); + } + } + + // If there are still more channels being created than there are free ones, + // then we take them away from the most greedy channel kind. + while (totalWantToAddCalculator() > freeChannelCount) { + Sort( + std::begin(wantToAddByKind), + std::end(wantToAddByKind), + [](const int* lhs, const int* rhs) { return *lhs > *rhs; }); + --(*wantToAddByKind[0]); + Y_DEBUG_ABORT_UNLESS(*wantToAddByKind[0] >= 0); + } +} + } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/core/volume_model.h b/cloud/blockstore/libs/storage/core/volume_model.h index e7e4a83b41..2c36c5f93b 100644 --- a/cloud/blockstore/libs/storage/core/volume_model.h +++ b/cloud/blockstore/libs/storage/core/volume_model.h @@ -98,4 +98,10 @@ ui64 ComputeMaxBlocks( const NCloud::NProto::EStorageMediaKind mediaKind, ui32 currentPartitions); +void ComputeChannelCountLimits( + int freeChannelCount, + int* wantToAddMerged, + int* wantToAddMixed, + int* wantToAddFresh); + } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/core/volume_model_ut.cpp b/cloud/blockstore/libs/storage/core/volume_model_ut.cpp index f14c7203e7..4475be48af 100644 --- a/cloud/blockstore/libs/storage/core/volume_model_ut.cpp +++ b/cloud/blockstore/libs/storage/core/volume_model_ut.cpp @@ -1342,6 +1342,56 @@ Y_UNIT_TEST_SUITE(TVolumeModelTest) ); } + Y_UNIT_TEST(ShouldNotOverflowWhenUseSeparatedMixedChannelCount) + { + NProto::TStorageServiceConfig storageServiceConfig; + storageServiceConfig.SetAllocateSeparateMixedChannels(true); + storageServiceConfig.SetHybridMixedChannelPoolKind("mixed"); + storageServiceConfig.SetHybridMergedChannelPoolKind("merged"); + storageServiceConfig.SetHybridFreshChannelPoolKind("fresh"); + storageServiceConfig.SetMinChannelCount(1); + storageServiceConfig.SetFreshChannelCount(1); + storageServiceConfig.SetAllocationUnitHDD(1); + storageServiceConfig.SetHDDUnitWriteIops(10); + storageServiceConfig.SetSSDUnitWriteIops(100); + storageServiceConfig.SetHDDUnitWriteBandwidth(10); + storageServiceConfig.SetSSDUnitWriteBandwidth(30); + storageServiceConfig.SetThrottlingEnabled(true); + + auto config = std::make_unique( + std::move(storageServiceConfig), + std::make_shared( + NCloud::NProto::TFeaturesConfig()) + ); + + const auto blocksCount = 400_GB / DefaultBlockSize; + TVolumeParams params{ + DefaultBlockSize, + blocksCount, + 1, + {}, + 0, + 0, + 0, + 0, + NCloud::NProto::STORAGE_MEDIA_HYBRID + }; + NKikimrBlockStore::TVolumeConfig volumeConfig; + ResizeVolume(*config, params, {}, {}, volumeConfig); + + auto countChannels = [&](EChannelDataKind kind) + { + return CountIf( + volumeConfig.GetExplicitChannelProfiles(), + [kind](const auto& channel) + { return channel.GetDataKind() == static_cast(kind); }); + }; + UNIT_ASSERT_VALUES_EQUAL(255, volumeConfig.ExplicitChannelProfilesSize()); + UNIT_ASSERT_VALUES_EQUAL(216, countChannels(EChannelDataKind::Merged)); + UNIT_ASSERT_VALUES_EQUAL(35, countChannels(EChannelDataKind::Mixed)); + UNIT_ASSERT_VALUES_EQUAL(1, countChannels(EChannelDataKind::Fresh)); + } + Y_UNIT_TEST(ShouldNotDecreaseDataChannelCount) { NProto::TStorageServiceConfig storageServiceConfig; @@ -1902,6 +1952,122 @@ Y_UNIT_TEST_SUITE(TVolumeModelTest) #undef CHECK_CHANNEL #undef CHECK_CHANNEL_HDD + + Y_UNIT_TEST(ShouldNotCapWhenEnoughChannels) + { + int wantAddMerged = 50; + int wantAddMixed = 35; + int wantAddFresh = 15; + + ComputeChannelCountLimits( + 100, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + + UNIT_ASSERT_VALUES_EQUAL(50, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(35, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(15, wantAddFresh); + } + + Y_UNIT_TEST(ShouldCapAllWhenNoFreeChannels) + { + int wantAddMerged = 50; + int wantAddMixed = 35; + int wantAddFresh = 15; + + ComputeChannelCountLimits( + 0, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + + UNIT_ASSERT_VALUES_EQUAL(0, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(0, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(0, wantAddFresh); + } + + Y_UNIT_TEST(ShouldCapLessAffectedChannel) + { + int wantAddMerged = 10; + int wantAddMixed = 1; + int wantAddFresh = 1; + + ComputeChannelCountLimits( + 2, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + UNIT_ASSERT_VALUES_EQUAL(0, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddFresh); + } + + Y_UNIT_TEST(ShouldDoFairCapping) + { + int wantAddMerged = 10; + int wantAddMixed = 100; + int wantAddFresh = 1; + + ComputeChannelCountLimits( + 101, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + UNIT_ASSERT_VALUES_EQUAL(9, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(91, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddFresh); + } + + Y_UNIT_TEST(ShouldGiveMoreToThoseWhoAskForMore) + { + int wantAddMerged = 20; + int wantAddMixed = 2; + int wantAddFresh = 1; + + ComputeChannelCountLimits( + 11, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + + UNIT_ASSERT_VALUES_EQUAL(9, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddFresh); + } + + Y_UNIT_TEST(ShouldGiveAtLeastOneChannel) + { + int wantAddMerged = 2000; + int wantAddMixed = 2; + int wantAddFresh = 1; + + ComputeChannelCountLimits( + 5, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + + UNIT_ASSERT_VALUES_EQUAL(3, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddFresh); + } + + Y_UNIT_TEST(ShouldGiveTheChannelWhereItIsMostNeeded) + { + int wantAddMerged = 2000; + int wantAddMixed = 2000; + int wantAddFresh = 1; + + ComputeChannelCountLimits( + 1, + &wantAddMerged, + &wantAddMixed, + &wantAddFresh); + UNIT_ASSERT_VALUES_EQUAL(0, wantAddMerged); + UNIT_ASSERT_VALUES_EQUAL(0, wantAddMixed); + UNIT_ASSERT_VALUES_EQUAL(1, wantAddFresh); + } } } // namespace NCloud::NBlockStore::NStorage