Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Dmitry Razumov committed Jan 2, 2025
1 parent 3bab816 commit bb7d7dd
Show file tree
Hide file tree
Showing 8 changed files with 222 additions and 175 deletions.
93 changes: 38 additions & 55 deletions cloud/blockstore/libs/storage/core/disk_counters.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,54 +377,40 @@ struct THistogramRequestCounters

explicit THistogramRequestCounters(
EHistogramCounterOptions histCounterOptions)
: Flush(EPublishingPolicy::Repl, histCounterOptions)
, AddBlobs(EPublishingPolicy::Repl, histCounterOptions)
, Compaction(EPublishingPolicy::Repl, histCounterOptions)
, Cleanup(EPublishingPolicy::Repl, histCounterOptions)
, CollectGarbage(EPublishingPolicy::Repl, histCounterOptions)
, DeleteGarbage(EPublishingPolicy::Repl, histCounterOptions)
, TrimFreshLog(EPublishingPolicy::Repl, histCounterOptions)
, AddConfirmedBlobs(EPublishingPolicy::Repl, histCounterOptions)
, AddUnconfirmedBlobs(EPublishingPolicy::Repl, histCounterOptions)
, ConfirmBlobs(EPublishingPolicy::Repl, histCounterOptions)
, WriteBlob(
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
histCounterOptions)
, ReadBlob(
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
histCounterOptions)
, PatchBlob(
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
histCounterOptions)
, ReadBlocks(
EPublishingPolicy::All,
ERequestCounterOption::HasVoidBytes,
histCounterOptions)
, WriteBlocks(EPublishingPolicy::All, histCounterOptions)
, ZeroBlocks(EPublishingPolicy::All, histCounterOptions)
, DescribeBlocks(EPublishingPolicy::All, histCounterOptions)
, ChecksumBlocks(EPublishingPolicy::All, histCounterOptions)
: HistCounterOptions(histCounterOptions)
{}

EHistogramCounterOptions HistCounterOptions;

// BlobStorage based
TLowResCounter Flush;
TLowResCounter AddBlobs;
TLowResCounter Compaction;
TLowResCounter Cleanup;
TLowResCounter CollectGarbage;
TLowResCounter DeleteGarbage;
TLowResCounter TrimFreshLog;
TLowResCounter AddConfirmedBlobs;
TLowResCounter AddUnconfirmedBlobs;
TLowResCounter ConfirmBlobs;
TLowResCounter Flush{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter AddBlobs{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter Compaction{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter Cleanup{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter CollectGarbage{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter DeleteGarbage{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter TrimFreshLog{EPublishingPolicy::Repl, HistCounterOptions};
TLowResCounter AddConfirmedBlobs{
EPublishingPolicy::Repl,
HistCounterOptions};
TLowResCounter AddUnconfirmedBlobs{
EPublishingPolicy::Repl,
HistCounterOptions};
TLowResCounter ConfirmBlobs{EPublishingPolicy::Repl, HistCounterOptions};

// BlobStorage based with kind and size
TLowResCounter WriteBlob;
TLowResCounter ReadBlob;
TLowResCounter PatchBlob;
TLowResCounter WriteBlob{
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
HistCounterOptions};
TLowResCounter ReadBlob{
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
HistCounterOptions};
TLowResCounter PatchBlob{
EPublishingPolicy::Repl,
ERequestCounterOption::HasKind,
HistCounterOptions};

static constexpr TLowResMeta AllLowResCounters[] = {
MakeMeta<&THistogramRequestCounters::Flush>(),
Expand All @@ -445,17 +431,12 @@ struct THistogramRequestCounters

THighResCounter ReadBlocks{
EPublishingPolicy::All,
ERequestCounterOption::HasVoidBytes};
THighResCounter WriteBlocks{
EPublishingPolicy::All,
};
THighResCounter ZeroBlocks{EPublishingPolicy::All};
THighResCounter DescribeBlocks{
EPublishingPolicy::All,
};
THighResCounter ChecksumBlocks{
EPublishingPolicy::All,
};
ERequestCounterOption::HasVoidBytes,
HistCounterOptions};
THighResCounter WriteBlocks{EPublishingPolicy::All, HistCounterOptions};
THighResCounter ZeroBlocks{EPublishingPolicy::All, HistCounterOptions};
THighResCounter DescribeBlocks{EPublishingPolicy::All, HistCounterOptions};
THighResCounter ChecksumBlocks{EPublishingPolicy::All, HistCounterOptions};

static constexpr THighResMeta AllHighResCounters[] = {
MakeMeta<&THistogramRequestCounters::ReadBlocks>(),
Expand All @@ -468,7 +449,9 @@ struct THistogramRequestCounters

static_assert(
sizeof(THistogramRequestCounters) ==
(sizeof(THistogramRequestCounters::TLowResCounter) *
// cannot use sizeof(EHistogramCounterOptions) because of alignment
(offsetof(THistogramRequestCounters, Flush) +
sizeof(THistogramRequestCounters::TLowResCounter) *
std::size(THistogramRequestCounters::AllLowResCounters) +
sizeof(THistogramRequestCounters::THighResCounter) *
std::size(THistogramRequestCounters::AllHighResCounters)));
Expand Down
158 changes: 92 additions & 66 deletions cloud/blockstore/libs/storage/core/histogram_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,52 @@
#include <library/cpp/testing/unittest/registar.h>

namespace NCloud::NBlockStore::NStorage {
using TDynamicCounterPtr = NMonitoring::TDynamicCounterPtr;
using TDynamicCounters = NMonitoring::TDynamicCounters;

////////////////////////////////////////////////////////////////////////////////

using TDynamicCounterPtr = NMonitoring::TDynamicCounterPtr;
using TDynamicCounters = NMonitoring::TDynamicCounters;
namespace {

////////////////////////////////////////////////////////////////////////////////

auto CreateTestHistogram1(
const TDynamicCounterPtr& group,
EHistogramCounterOptions histCounterOptions)
{
THistogram<TKbSizeBuckets> histogram(histCounterOptions);
histogram.Register(group, true);

histogram.Increment(1);
histogram.Increment(2, 1);
histogram.Increment(4, 3); // 4KB: 5
histogram.Increment(6, 1);
histogram.Increment(7, 3); // 8KB: 4
histogram.Increment(10, 1);
histogram.Increment(16, 2); // 16KB: 3
histogram.Increment(20, 1);
histogram.Increment(30, 1); // 32KB: 2
histogram.Increment(40, 1); // 64KB: 1
return histogram;
}
auto CreateTestHistogram1(
const TDynamicCounterPtr& group,
NCloud::EHistogramCounterOptions histCounterOptions)
{
THistogram<TKbSizeBuckets> histogram(histCounterOptions);
histogram.Register(group, true);

histogram.Increment(1);
histogram.Increment(2, 1);
histogram.Increment(4, 3); // 4KB: 5
histogram.Increment(6, 1);
histogram.Increment(7, 3); // 8KB: 4
histogram.Increment(10, 1);
histogram.Increment(16, 2); // 16KB: 3
histogram.Increment(20, 1);
histogram.Increment(30, 1); // 32KB: 2
histogram.Increment(40, 1); // 64KB: 1
return histogram;
}

auto CreateTestHistogram2(
const TDynamicCounterPtr& group,
EHistogramCounterOptions histCounterOptions)
{
THistogram<TKbSizeBuckets> histogram(histCounterOptions);
histogram.Register(group, true);

histogram.Increment(1024, 1);
histogram.Increment(2000, 2);
histogram.Increment(3000, 2);
histogram.Increment(4096, 1);
histogram.Increment(4097, 1);
histogram.Increment(10000, 1);
histogram.Increment(100500, 1);
histogram.Increment(1000000000, 1);
return histogram;
}
auto CreateTestHistogram2(
const TDynamicCounterPtr& group,
EHistogramCounterOptions histCounterOptions)
{
THistogram<TKbSizeBuckets> histogram(histCounterOptions);
histogram.Register(group, true);

histogram.Increment(1024, 1);
histogram.Increment(2000, 2);
histogram.Increment(3000, 2);
histogram.Increment(4096, 1);
histogram.Increment(4097, 1);
histogram.Increment(10000, 1);
histogram.Increment(100500, 1);
histogram.Increment(1000000000, 1);
return histogram;
}

} // namespace

////////////////////////////////////////////////////////////////////////////////

Expand All @@ -70,7 +71,10 @@ Y_UNIT_TEST_SUITE(THistogramTest)

const TVector<ui64> expected = {5, 4, 3, 2, 1, 0, 0, 0, 0, 0, 0, 0};
for (size_t i = 0; i < buckets.size(); ++i) {
UNIT_ASSERT_VALUES_EQUAL_C(buckets[i], expected[i], "index=" + ToString(i));
UNIT_ASSERT_VALUES_EQUAL_C(
buckets[i],
expected[i],
"index=" + ToString(i));
}
}

Expand All @@ -82,7 +86,10 @@ Y_UNIT_TEST_SUITE(THistogramTest)

const TVector<ui64> expected = {0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4};
for (size_t i = 0; i < buckets.size(); ++i) {
UNIT_ASSERT_VALUES_EQUAL_C(buckets[i], expected[i], "index=" + ToString(i));
UNIT_ASSERT_VALUES_EQUAL_C(
buckets[i],
expected[i],
"index=" + ToString(i));
}
}
}
Expand All @@ -97,7 +104,9 @@ Y_UNIT_TEST_SUITE(THistogramTest)
EHistogramCounterOption::ReportMultipleCounters);
const auto buckets = histogram.GetPercentileBuckets();

UNIT_ASSERT_VALUES_EQUAL(buckets.size(), TKbSizeBuckets::BUCKETS_COUNT);
UNIT_ASSERT_VALUES_EQUAL(
buckets.size(),
TKbSizeBuckets::BUCKETS_COUNT);
UNIT_ASSERT_DOUBLES_EQUAL(buckets[0].first, 4, Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(buckets[1].first, 8, Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(buckets[2].first, 16, Min<double>());
Expand All @@ -116,11 +125,16 @@ Y_UNIT_TEST_SUITE(THistogramTest)
EHistogramCounterOption::ReportMultipleCounters);
const auto buckets = histogram.GetPercentileBuckets();

UNIT_ASSERT_VALUES_EQUAL(buckets.size(), TKbSizeBuckets::BUCKETS_COUNT);
UNIT_ASSERT_VALUES_EQUAL(
buckets.size(),
TKbSizeBuckets::BUCKETS_COUNT);
UNIT_ASSERT_DOUBLES_EQUAL(buckets[8].first, 1024, Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(buckets[9].first, 2048, Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(buckets[10].first, 4096, Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(buckets[11].first, Max<double>(), Min<double>());
UNIT_ASSERT_DOUBLES_EQUAL(
buckets[11].first,
Max<double>(),
Min<double>());
UNIT_ASSERT_VALUES_EQUAL(buckets[8].second, 1);
UNIT_ASSERT_VALUES_EQUAL(buckets[9].second, 2);
UNIT_ASSERT_VALUES_EQUAL(buckets[10].second, 3);
Expand All @@ -132,26 +146,26 @@ Y_UNIT_TEST_SUITE(THistogramTest)
{
TDynamicCounterPtr group{new TDynamicCounters()};
auto histogram = CreateTestHistogram1(
group,
EHistogramCounterOption::ReportMultipleCounters);
group,
EHistogramCounterOption::ReportMultipleCounters);

TVector<double> percentiles = histogram.CalculatePercentiles();
UNIT_ASSERT_VALUES_EQUAL(percentiles.size(), 5);
UNIT_ASSERT_DOUBLES_EQUAL(percentiles[0], 6.5, 0.1); // p50
UNIT_ASSERT_VALUES_EQUAL(percentiles[4], 64); // p100
UNIT_ASSERT_DOUBLES_EQUAL(percentiles[0], 6.5, 0.1); // p50
UNIT_ASSERT_VALUES_EQUAL(percentiles[4], 64); // p100
}

Y_UNIT_TEST(TestReset)
{
TDynamicCounterPtr group{new TDynamicCounters()};
auto histogram = CreateTestHistogram1(
group,
EHistogramCounterOption::ReportMultipleCounters);
group,
EHistogramCounterOption::ReportMultipleCounters);

histogram.Reset();

const auto buckets = histogram.GetSolomonHistogram();
for (auto value : buckets) {
for (auto value: buckets) {
UNIT_ASSERT_VALUES_EQUAL(value, 0);
}
}
Expand All @@ -160,20 +174,23 @@ Y_UNIT_TEST_SUITE(THistogramTest)
{
TDynamicCounterPtr group{new TDynamicCounters()};
auto histogram1 = CreateTestHistogram1(
group,
EHistogramCounterOption::ReportMultipleCounters);
group,
EHistogramCounterOption::ReportMultipleCounters);
auto histogram2 = CreateTestHistogram2(
group,
EHistogramCounterOption::ReportMultipleCounters);
group,
EHistogramCounterOption::ReportMultipleCounters);
auto histogram3 = CreateTestHistogram1(
group,
EHistogramCounterOption::ReportMultipleCounters);
group,
EHistogramCounterOption::ReportMultipleCounters);

{
const TVector<ui64> expected = {5, 4, 3, 2, 1, 0, 0, 0, 0, 0, 0, 0};
const auto buckets = histogram1.GetSolomonHistogram();
for (size_t i = 0; i < buckets.size(); ++i) {
UNIT_ASSERT_VALUES_EQUAL_C(buckets[i], expected[i], "index=" + ToString(i));
UNIT_ASSERT_VALUES_EQUAL_C(
buckets[i],
expected[i],
"index=" + ToString(i));
}
}

Expand All @@ -182,16 +199,23 @@ Y_UNIT_TEST_SUITE(THistogramTest)
const TVector<ui64> expected = {5, 4, 3, 2, 1, 0, 0, 0, 1, 2, 3, 4};
const auto buckets = histogram1.GetSolomonHistogram();
for (size_t i = 0; i < buckets.size(); ++i) {
UNIT_ASSERT_VALUES_EQUAL_C(buckets[i], expected[i], "index=" + ToString(i));
UNIT_ASSERT_VALUES_EQUAL_C(
buckets[i],
expected[i],
"index=" + ToString(i));
}
}

histogram1.Add(histogram3);
{
const TVector<ui64> expected = {10, 8, 6, 4, 2, 0, 0, 0, 1, 2, 3, 4};
const TVector<ui64> expected =
{10, 8, 6, 4, 2, 0, 0, 0, 1, 2, 3, 4};
const auto buckets = histogram1.GetSolomonHistogram();
for (size_t i = 0; i < buckets.size(); ++i) {
UNIT_ASSERT_VALUES_EQUAL_C(buckets[i], expected[i], "index=" + ToString(i));
UNIT_ASSERT_VALUES_EQUAL_C(
buckets[i],
expected[i],
"index=" + ToString(i));
}
}
}
Expand All @@ -215,7 +239,8 @@ Y_UNIT_TEST_SUITE(THistogramTest)
UNIT_ASSERT_STRINGS_EQUAL(ss.Str(), expected);
}

Y_UNIT_TEST(ShouldReportHistogramMultipleCounters) {
Y_UNIT_TEST(ShouldReportHistogramMultipleCounters)
{
TDynamicCounterPtr group(new TDynamicCounters());
auto histogram1 = CreateTestHistogram1(
group,
Expand Down Expand Up @@ -247,9 +272,10 @@ Y_UNIT_TEST_SUITE(THistogramTest)
Y_UNIT_TEST(ShouldReportHistogramBothSingleAndMultipleCounters)
{
TDynamicCounterPtr group(new TDynamicCounters());
auto histogram1 = CreateTestHistogram1(group,
auto histogram1 = CreateTestHistogram1(
group,
EHistogramCounterOption::ReportSingleCounter |
EHistogramCounterOption::ReportMultipleCounters);
EHistogramCounterOption::ReportMultipleCounters);
TStringStream ss;
TCountersPrinter printer(&ss);
group->Accept("root", "counters", printer);
Expand All @@ -276,4 +302,4 @@ Y_UNIT_TEST_SUITE(THistogramTest)
}
}

} // namespace NCloud::NBlockStore::NStorage
} // namespace NCloud::NBlockStore::NStorage
Loading

0 comments on commit bb7d7dd

Please sign in to comment.