Skip to content

Commit

Permalink
Fix channel count overflow (#1645)
Browse files Browse the repository at this point in the history 
* Fix channel count overflow

* Fix review issues

* Fix params mess

* Making calculations simpler

* Remove unused param
  • Loading branch information
@drbasic
drbasic committed Aug 9, 2024
1 parent a5da82c commit 863cc65
Show file tree
Hide file tree
Showing 3 changed files with 242 additions and 0 deletions.
70 changes: 70 additions & 0 deletions cloud/blockstore/libs/storage/core/volume_model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

#include <ydb/core/protos/blockstore_config.pb.h>

#include <util/generic/algorithm.h>
#include <util/generic/cast.h>
#include <util/generic/size_literals.h>
#include <util/generic/ymath.h>

#include <cmath>

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<double>(*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
6 changes: 6 additions & 0 deletions cloud/blockstore/libs/storage/core/volume_model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
166 changes: 166 additions & 0 deletions cloud/blockstore/libs/storage/core/volume_model_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TStorageConfig>(
std::move(storageServiceConfig),
std::make_shared<NFeatures::TFeaturesConfig>(
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<ui32>(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;
Expand Down Expand Up @@ -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

0 comments on commit 863cc65

Please sign in to comment.