Skip to content

Commit 79f8738

Browse files
gy2411gy2411
committed
fixes, add tests
1 parent 6ec6f2f commit 79f8738

File tree

5 files changed

+92
-55
lines changed

5 files changed

+92
-55
lines changed

‎cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,6 @@ void TCompactionActor::Bootstrap(const TActorContext& ctx)
272272

273273
void TCompactionActor::ProcessRequests(const TActorContext& ctx)
274274
{
275-
// TODO:_ TRequestScope timer(*RequestInfo) here?
276-
// TODO:_ lwtrack here?
277-
278275
if (Requests) {
279276
ReadBlocks(ctx);
280277

@@ -865,8 +862,6 @@ void TCompactionActor::HandleCompactionTxResponse(
865862
{
866863
auto* msg = ev->Get();
867864

868-
// TODO:_ exec cycles?
869-
870865
if (HandleError(ctx, msg->GetError())) {
871866
return;
872867
}
@@ -885,25 +880,6 @@ void TCompactionActor::HandleCompactionTxResponse(
885880
return;
886881
}
887882

888-
// TODO:_ remove
889-
// TVector<TRangeCompactionInfo*> infos;
890-
// infos.Reserve(RangeCompactionInfos.size())
891-
// for (auto& info: RangeCompactionInfos) {
892-
// infos.push_back(&info);
893-
// }
894-
// Sort(
895-
// RangeCompactionInfos,
896-
// [](const TRangeCompactionInfo& l, const TRangeCompactionInfo& r)
897-
// { return l->RangeCompactionIndex < r->RangeCompactionIndex; });
898-
899-
// TODO:_ remove
900-
// for (ui32 i = 0; i < infos.size(); ++i) {
901-
// RangeCompactionInfos.push_back(
902-
// std::move(infos[i]->first));
903-
// }
904-
905-
// TODO:_ exec cycles
906-
907883
for (auto context: ForkedReadCallContexts) {
908884
RequestInfo->CallContext->LWOrbit.Join(context->LWOrbit);
909885
}

‎cloud/blockstore/libs/storage/partition/part_actor_compactiontx.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,17 +609,13 @@ void TPartitionActor::CompleteCompaction(
609609
}
610610
}
611611

612-
// TODO:_ or to it in the beginning of the Complete... function?
613612
TRequestScope timer(*args.RequestInfo);
614613

615614
auto response =
616615
std::make_unique<TEvPartitionPrivate::TEvCompactionTxResponse>(
617616
std::move(rangeCompactionInfos),
618617
std::move(requests));
619618

620-
// TODO:_ ExecCycles?
621-
// response->ExecCycles = args.RequestInfo->GetExecCycles();
622-
623619
LWTRACK(
624620
ResponseSent_Partition,
625621
args.RequestInfo->CallContext->LWOrbit,

‎cloud/blockstore/libs/storage/partition/part_compaction.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#pragma once
22

33
#include <cloud/blockstore/libs/common/block_range.h>
4-
#include <cloud/blockstore/libs/storage/model/channel_data_kind.h> // TODO:_ ???
4+
#include <cloud/blockstore/libs/storage/model/channel_data_kind.h>
55
#include <cloud/blockstore/libs/storage/partition/model/affected.h>
66
#include <cloud/blockstore/libs/storage/partition/model/block_mask.h>
77

@@ -10,8 +10,8 @@
1010
#include <cloud/storage/core/libs/tablet/model/partial_blob_id.h>
1111

1212
#include <contrib/ydb/core/base/blobstorage.h>
13-
#include <contrib/ydb/core/protos/blobstorage.pb.h> // TODO:_ ???
14-
#include <contrib/ydb/library/actors/core/actorid.h> // TODO:_ ???
13+
#include <contrib/ydb/core/protos/blobstorage.pb.h>
14+
#include <contrib/ydb/library/actors/core/actorid.h>
1515

1616
#include <util/generic/array_ref.h>
1717
#include <util/generic/ptr.h>

‎cloud/blockstore/libs/storage/partition/part_events_private.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#pragma once
22

3-
#include "part_compaction.h" // TODO:_ is it ok? Should we move it to model? Or rearrange dependencies?
3+
#include "part_compaction.h"
44
#include "public.h"
55

66
#include <cloud/blockstore/libs/common/block_range.h>
@@ -471,11 +471,10 @@ struct TEvPartitionPrivate
471471
const ui32 RangeCompactionIndex;
472472
const TCompactionOptions CompactionOptions;
473473
TVector<TCompactionRange> Ranges;
474-
// TVector<std::pair<ui32, TBlockRange32>> Ranges; // TODO:_ use using?
475474

476-
TCompactionTxRequest() = default; // TODO:_ do we need it?
475+
TCompactionTxRequest() = default;
477476

478-
TCompactionTxRequest( // TODO:_ do we need it?
477+
TCompactionTxRequest(
479478
ui64 commitId,
480479
ui32 rangeCompactionIndex,
481480
TCompactionOptions compactionOptions,
@@ -492,9 +491,9 @@ struct TEvPartitionPrivate
492491
TVector<TRangeCompactionInfo> RangeCompactionInfos;
493492
TVector<TCompactionReadRequest> Requests;
494493

495-
TCompactionTxResponse() = default; // TODO:_ do we need it?
494+
TCompactionTxResponse() = default;
496495

497-
TCompactionTxResponse( // TODO:_ do we need it?
496+
TCompactionTxResponse(
498497
TVector<TRangeCompactionInfo> rangeCompactionInfos,
499498
TVector<TCompactionReadRequest> requests)
500499
: RangeCompactionInfos(std::move(rangeCompactionInfos))

‎cloud/blockstore/libs/storage/partition/part_ut.cpp‎

Lines changed: 84 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8443,11 +8443,14 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
84438443
UNIT_ASSERT(suicideHappened);
84448444
}
84458445

8446-
Y_UNIT_TEST(ShouldProcessMultipleRangesUponCompaction)
8446+
void DoShouldProcessMultipleRangesUponCompaction(
8447+
bool splitTxInBatchCompactionEnabled)
84478448
{
84488449
auto config = DefaultConfig();
84498450
config.SetWriteBlobThreshold(1_MB);
84508451
config.SetBatchCompactionEnabled(true);
8452+
config.SetSplitTxInBatchCompactionEnabled(
8453+
splitTxInBatchCompactionEnabled);
84518454
config.SetCompactionRangeCountPerRun(3);
84528455
config.SetSSDMaxBlobsPerRange(999);
84538456
config.SetHDDMaxBlobsPerRange(999);
@@ -8542,6 +8545,12 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
85428545
}
85438546
}
85448547

8548+
Y_UNIT_TEST(ShouldProcessMultipleRangesUponCompaction)
8549+
{
8550+
DoShouldProcessMultipleRangesUponCompaction(true);
8551+
DoShouldProcessMultipleRangesUponCompaction(false);
8552+
}
8553+
85458554
Y_UNIT_TEST(ShouldPatchBlobsDuringCompaction)
85468555
{
85478556
auto config = DefaultConfig();
@@ -10006,11 +10015,14 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1000610015
ui32 increasingPercentageThreshold,
1000710016
ui32 decreasingPercentageThreshold,
1000810017
ui32 compactionRangeCountPerRun,
10009-
ui32 maxCompactionRangeCountPerRun = 10)
10018+
ui32 maxCompactionRangeCountPerRun = 10,
10019+
bool splitTxInBatchCompactionEnabled = true)
1001010020
{
1001110021
auto config = DefaultConfig();
1001210022
config.SetWriteBlobThreshold(1_MB);
1001310023
config.SetBatchCompactionEnabled(true);
10024+
config.SetSplitTxInBatchCompactionEnabled(
10025+
splitTxInBatchCompactionEnabled);
1001410026
config.SetV1GarbageCompactionEnabled(true);
1001510027
config.SetCompactionGarbageThreshold(diskGarbageThreshold);
1001610028
config.SetCompactionRangeGarbageThreshold(rangeGarbageThreshold);
@@ -10107,7 +10119,9 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1010710119
// blobs percentage: (10 - 9 / 10) * 100 = 10
1010810120
// 10 < 100, so batch size should decrement
1010910121
CheckIncrementAndDecrementCompactionPerRun(
10110-
2, 9, 999999, 999999, 99999, 7, 200, 100, 1);
10122+
2, 9, 999999, 999999, 99999, 7, 200, 100, 1, 10, true);
10123+
CheckIncrementAndDecrementCompactionPerRun(
10124+
2, 9, 999999, 999999, 99999, 7, 200, 100, 1, 10, false);
1011110125
}
1011210126

1011310127
Y_UNIT_TEST(ShouldChangeBatchSizeDueToBlocksPerDisk)
@@ -10120,11 +10134,15 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1012010134

1012110135
// 9 > 8, so should increment and compact 2 ranges
1012210136
CheckIncrementAndDecrementCompactionPerRun(
10123-
1, 1000, 99999, 203, 99999, 5, 8, 5, 2);
10137+
1, 1000, 99999, 203, 99999, 5, 8, 5, 2, 10, true);
10138+
CheckIncrementAndDecrementCompactionPerRun(
10139+
1, 1000, 99999, 203, 99999, 5, 8, 5, 2, 10, false);
1012410140

1012510141
// 9 < 15, so should decrement and compact only 1 range
1012610142
CheckIncrementAndDecrementCompactionPerRun(
10127-
2, 1000, 99999, 203, 99999, 7, 30, 15, 1);
10143+
2, 1000, 99999, 203, 99999, 7, 30, 15, 1, 10, true);
10144+
CheckIncrementAndDecrementCompactionPerRun(
10145+
2, 1000, 99999, 203, 99999, 7, 30, 15, 1, 10, false);
1012810146
}
1012910147

1013010148
Y_UNIT_TEST(ShouldChangeBatchSizeDueToBlocksPerRangeCount)
@@ -10136,29 +10154,40 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1013610154

1013710155
// 7 > 6, so should increment and compact 2 ranges
1013810156
CheckIncrementAndDecrementCompactionPerRun(
10139-
1, 1000, 99999, 9999, 280, 5, 6, 4, 2);
10157+
1, 1000, 99999, 9999, 280, 5, 6, 4, 2, 10, true);
10158+
CheckIncrementAndDecrementCompactionPerRun(
10159+
1, 1000, 99999, 9999, 280, 5, 6, 4, 2, 10, false);
1014010160

1014110161
// 7 > 6, but should compact only 1 range due to maxRangeCountPerRun
1014210162
CheckIncrementAndDecrementCompactionPerRun(
10143-
1, 1000, 99999, 9999, 280, 7, 6, 4, 1, 1);
10163+
1, 1000, 99999, 9999, 280, 7, 6, 4, 1, 1, true);
10164+
CheckIncrementAndDecrementCompactionPerRun(
10165+
1, 1000, 99999, 9999, 280, 7, 6, 4, 1, 1, false);
1014410166

1014510167
// 7 < 8, so should decrement and compact only 1 range
1014610168
CheckIncrementAndDecrementCompactionPerRun(
10147-
2, 1000, 99999, 9999, 280, 7, 30, 8, 1);
10169+
2, 1000, 99999, 9999, 280, 7, 30, 8, 1, 10, true);
10170+
CheckIncrementAndDecrementCompactionPerRun(
10171+
2, 1000, 99999, 9999, 280, 7, 30, 8, 1, 10, false);
1014810172
}
1014910173

1015010174
Y_UNIT_TEST(ShouldDecrementBatchSizeWhenBlobsPerRangeCountIsSmall) {
1015110175
// CompactionScore in range3: 4 - 4 + eps
1015210176
// 100 * (eps - 0) / 1024 < 3, so should decrement and compact 1 range
1015310177
CheckIncrementAndDecrementCompactionPerRun(
10154-
2, 1000, 4, 9999, 9999, 7, 50, 10, 1);
10178+
2, 1000, 4, 9999, 9999, 7, 50, 10, 1, 10, true);
10179+
CheckIncrementAndDecrementCompactionPerRun(
10180+
2, 1000, 4, 9999, 9999, 7, 50, 10, 1, 10, false);
1015510181
}
1015610182

10157-
Y_UNIT_TEST(ShouldRespectCompactionCountPerRunChangingPeriod)
10183+
void DoShouldRespectCompactionCountPerRunChangingPeriod(
10184+
bool splitTxInBatchCompactionEnabled)
1015810185
{
1015910186
auto config = DefaultConfig();
1016010187
config.SetWriteBlobThreshold(1_MB);
1016110188
config.SetBatchCompactionEnabled(true);
10189+
config.SetSplitTxInBatchCompactionEnabled(
10190+
splitTxInBatchCompactionEnabled);
1016210191
config.SetV1GarbageCompactionEnabled(true);
1016310192
config.SetCompactionGarbageThreshold(99999);
1016410193
config.SetCompactionRangeGarbageThreshold(280);
@@ -10265,6 +10294,12 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1026510294
}
1026610295
}
1026710296

10297+
Y_UNIT_TEST(ShouldRespectCompactionCountPerRunChangingPeriod)
10298+
{
10299+
DoShouldRespectCompactionCountPerRunChangingPeriod(true);
10300+
DoShouldRespectCompactionCountPerRunChangingPeriod(false);
10301+
}
10302+
1026810303
template <typename FSend, typename FReceive>
1026910304
void DoShouldReportLongRunningBlobOperations(
1027010305
FSend sendRequest,
@@ -11237,10 +11272,13 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1123711272
UNIT_ASSERT_VALUES_EQUAL(E_TRY_AGAIN, response->GetStatus());
1123811273
}
1123911274

11240-
Y_UNIT_TEST(ShouldProcessMultipleRangesUponGarbageCompaction)
11275+
void DoShouldProcessMultipleRangesUponGarbageCompaction(
11276+
bool splitTxInBatchCompactionEnabled)
1124111277
{
1124211278
auto config = DefaultConfig();
1124311279
config.SetBatchCompactionEnabled(true);
11280+
config.SetSplitTxInBatchCompactionEnabled(
11281+
splitTxInBatchCompactionEnabled);
1124411282
config.SetGarbageCompactionRangeCountPerRun(3);
1124511283
config.SetV1GarbageCompactionEnabled(true);
1124611284
config.SetCompactionGarbageThreshold(20);
@@ -11336,10 +11374,19 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1133611374
}
1133711375
}
1133811376

11339-
Y_UNIT_TEST(ShouldProcessMultipleRangesUponForceCompaction)
11377+
Y_UNIT_TEST(ShouldProcessMultipleRangesUponGarbageCompaction)
11378+
{
11379+
DoShouldProcessMultipleRangesUponGarbageCompaction(true);
11380+
DoShouldProcessMultipleRangesUponGarbageCompaction(false);
11381+
}
11382+
11383+
void DoShouldProcessMultipleRangesUponForceCompaction(
11384+
bool splitTxInBatchCompactionEnabled)
1134011385
{
1134111386
auto config = DefaultConfig();
1134211387
config.SetBatchCompactionEnabled(true);
11388+
config.SetSplitTxInBatchCompactionEnabled(
11389+
splitTxInBatchCompactionEnabled);
1134311390
config.SetForcedCompactionRangeCountPerRun(3);
1134411391
config.SetV1GarbageCompactionEnabled(false);
1134511392

@@ -11440,10 +11487,19 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1144011487
}
1144111488
}
1144211489

11443-
Y_UNIT_TEST(ShouldSkipEmptyRangesUponForcedCompactionWithMultipleRanges)
11490+
Y_UNIT_TEST(ShouldProcessMultipleRangesUponForceCompaction)
11491+
{
11492+
DoShouldProcessMultipleRangesUponForceCompaction(true);
11493+
DoShouldProcessMultipleRangesUponForceCompaction(false);
11494+
}
11495+
11496+
void DoShouldSkipEmptyRangesUponForcedCompactionWithMultipleRanges(
11497+
bool splitTxInBatchCompactionEnabled)
1144411498
{
1144511499
auto config = DefaultConfig();
1144611500
config.SetBatchCompactionEnabled(true);
11501+
config.SetSplitTxInBatchCompactionEnabled(
11502+
splitTxInBatchCompactionEnabled);
1144711503
config.SetForcedCompactionRangeCountPerRun(3);
1144811504
config.SetV1GarbageCompactionEnabled(false);
1144911505

@@ -11473,6 +11529,12 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1147311529
UNIT_ASSERT_EQUAL(5, compactionStatus->Record.GetTotal());
1147411530
}
1147511531

11532+
Y_UNIT_TEST(ShouldSkipEmptyRangesUponForcedCompactionWithMultipleRanges)
11533+
{
11534+
DoShouldSkipEmptyRangesUponForcedCompactionWithMultipleRanges(true);
11535+
DoShouldSkipEmptyRangesUponForcedCompactionWithMultipleRanges(false);
11536+
}
11537+
1147611538
Y_UNIT_TEST(ShouldBatchSmallWritesToFreshChannelIfThresholdNotExceeded)
1147711539
{
1147811540
NProto::TStorageServiceConfig config;
@@ -11838,10 +11900,14 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1183811900
}
1183911901
}
1184011902

11841-
void TestForcedCompaction(ui32 rangesPerRun)
11903+
void TestForcedCompaction(
11904+
ui32 rangesPerRun,
11905+
bool splitTxInBatchCompactionEnabled)
1184211906
{
1184311907
auto config = DefaultConfig();
1184411908
config.SetBatchCompactionEnabled(true);
11909+
config.SetSplitTxInBatchCompactionEnabled(
11910+
splitTxInBatchCompactionEnabled);
1184511911
config.SetForcedCompactionRangeCountPerRun(rangesPerRun);
1184611912
config.SetV1GarbageCompactionEnabled(false);
1184711913
config.SetWriteBlobThreshold(15_KB);
@@ -11960,12 +12026,14 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1196012026

1196112027
Y_UNIT_TEST(ShouldCompactSeveralBlobsInTheSameRangeWithOneRangePerRun)
1196212028
{
11963-
TestForcedCompaction(1);
12029+
TestForcedCompaction(1, true);
12030+
TestForcedCompaction(1, false);
1196412031
}
1196512032

1196612033
Y_UNIT_TEST(ShouldCompactSeveralBlobsInTheSameRangeWithSeveralRangesPerRun)
1196712034
{
11968-
TestForcedCompaction(10);
12035+
TestForcedCompaction(10, true);
12036+
TestForcedCompaction(10, false);
1196912037
}
1197012038

1197112039
Y_UNIT_TEST(ShouldWriteToMixedChannelOnHddIfThresholdExceeded)
@@ -13098,5 +13166,3 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
1309813166
}
1309913167

1310013168
} // namespace NCloud::NBlockStore::NStorage::NPartition
13101-
13102-
// TODO:_ add tests!!!

0 commit comments

Comments
 (0)