Skip to content

Commit 9a0ad2a

Browse files
Georg-0001Georg-0001
committed
fix remarks
1 parent 0466eb5 commit 9a0ad2a

File tree

4 files changed

+103
-93
lines changed

4 files changed

+103
-93
lines changed

‎cloud/blockstore/libs/storage/disk_agent/model/device_client.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ NCloud::NProto::TError TDeviceClient::ReleaseDevices(
262262
"Device %s was released by client %s for reading.",
263263
uuid.Quote().c_str(),
264264
clientId.c_str());
265-
deviceState->ReaderSessions = {};
265+
deviceState->ReaderSessions.clear();
266266
}
267267
}
268268

‎cloud/blockstore/libs/storage/testlib/disk_registry_proxy_mock.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,8 @@ class TDiskRegistryProxyMock final
434434
clientId == disk->WriterClientId || clientId == AnyWriterClientId)
435435
{
436436
disk->WriterClientId = "";
437+
} else if (clientId == AnyReaderClientId) {
438+
disk->ReaderClientIds.clear();
437439
} else {
438440
auto it = Find(
439441
disk->ReaderClientIds.begin(),

‎cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp

Lines changed: 58 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ constexpr auto ExternalDrainTimeout = TDuration::Seconds(30);
3434

3535
constexpr auto ReleaseShadowDiskRetryTimeout = TDuration::Seconds(5);
3636

37-
constexpr auto ReleaseForAnyWriter = 1;
38-
39-
constexpr auto ReleaseForAnyReader = 2;
40-
41-
constexpr auto ExternalDrainTimeoutTag = 3;
37+
enum class EReleaseSessionKind
38+
{
39+
AnyReader = 1,
40+
AnyWriter = 2,
41+
};
4242

4343
struct TPartitionDescr
4444
{
@@ -174,7 +174,8 @@ class TCheckpointActor final
174174
ui32 Responses = 0;
175175
NProto::TError Error;
176176
TString ShadowDiskId;
177-
bool OneReleaseShadowDiskRequestFinished = false;
177+
bool ReleaseShadowDiskRequestForAnyReaderFinished = false;
178+
bool ReleaseShadowDiskRequestForAnyWriterFinished = false;
178179

179180
TVector<TCallContextPtr> ChildCallContexts;
180181

@@ -215,7 +216,7 @@ class TCheckpointActor final
215216
void ForkTraces(TCallContextPtr callContext);
216217
void JoinTraces(ui32 cookie);
217218
std::unique_ptr<TEvDiskRegistry::TEvReleaseDiskRequest>
218-
MakeReleaseDiskRequest(const TString& clientId) const;
219+
MakeReleaseDiskRequest(EReleaseSessionKind releaseSessionKind) const;
219220

220221
private:
221222
STFUNC(StateDrain);
@@ -263,13 +264,9 @@ class TCheckpointActor final
263264
const TEvents::TEvPoisonPill::TPtr& ev,
264265
const TActorContext& ctx);
265266

266-
void HandleReleaseShadowDisk(
267+
void HandleReleaseDiskResponse(
267268
const TEvDiskRegistry::TEvReleaseDiskResponse::TPtr& ev,
268269
const TActorContext& ctx);
269-
270-
void HandleReleaseShadowDiskRetry(
271-
const TEvents::TEvWakeup::TPtr& ev,
272-
const TActorContext& ctx);
273270
};
274271

275272
////////////////////////////////////////////////////////////////////////////////
@@ -346,9 +343,7 @@ void TCheckpointActor<TMethod>::ExternalDrain(const TActorContext& ctx)
346343
"[%lu] Wait for external drain event",
347344
VolumeTabletId);
348345

349-
ctx.Schedule(
350-
ExternalDrainTimeout,
351-
new TEvents::TEvWakeup(ExternalDrainTimeoutTag));
346+
ctx.Schedule(ExternalDrainTimeout, new TEvents::TEvWakeup());
352347

353348
TBase::Become(&TThis::StateDrain);
354349
}
@@ -443,8 +438,20 @@ void TCheckpointActor<TMethod>::JoinTraces(ui32 cookie)
443438

444439
template <typename TMethod>
445440
std::unique_ptr<TEvDiskRegistry::TEvReleaseDiskRequest>
446-
TCheckpointActor<TMethod>::MakeReleaseDiskRequest(const TString& clientId) const
441+
TCheckpointActor<TMethod>::MakeReleaseDiskRequest(
442+
EReleaseSessionKind releaseSessionKind) const
447443
{
444+
TString clientId = "";
445+
446+
switch (releaseSessionKind) {
447+
case EReleaseSessionKind::AnyReader:
448+
clientId = AnyReaderClientId;
449+
break;
450+
case EReleaseSessionKind::AnyWriter:
451+
clientId = AnyWriterClientId;
452+
break;
453+
}
454+
448455
auto request = std::make_unique<TEvDiskRegistry::TEvReleaseDiskRequest>();
449456
request->Record.SetDiskId(ShadowDiskId);
450457
request->Record.MutableHeaders()->SetClientId(clientId);
@@ -521,19 +528,20 @@ void TCheckpointActor<TMethod>::HandleExternalDrainDone(
521528
VolumeTabletId);
522529

523530
ShadowDiskId = ev->Get()->ShadowDiskId;
524-
OneReleaseShadowDiskRequestFinished = false;
531+
ReleaseShadowDiskRequestForAnyReaderFinished = false;
532+
ReleaseShadowDiskRequestForAnyWriterFinished = false;
525533

526534
NCloud::Send(
527535
ctx,
528536
MakeDiskRegistryProxyServiceId(),
529-
MakeReleaseDiskRequest(TString(AnyWriterClientId)),
530-
ReleaseForAnyWriter);
537+
MakeReleaseDiskRequest(EReleaseSessionKind::AnyReader),
538+
static_cast<ui64>(EReleaseSessionKind::AnyReader));
531539

532540
NCloud::Send(
533541
ctx,
534542
MakeDiskRegistryProxyServiceId(),
535-
MakeReleaseDiskRequest(TString(AnyReaderClientId)),
536-
ReleaseForAnyReader);
543+
MakeReleaseDiskRequest(EReleaseSessionKind::AnyWriter),
544+
static_cast<ui64>(EReleaseSessionKind::AnyWriter));
537545

538546
TBase::Become(&TThis::StateReleaseShadowDisk);
539547
}
@@ -543,14 +551,11 @@ void TCheckpointActor<TMethod>::HandleExternalDrainTimeout(
543551
const TEvents::TEvWakeup::TPtr& ev,
544552
const TActorContext& ctx)
545553
{
546-
auto tag = ev->Get()->Tag;
547-
548-
if (tag != ExternalDrainTimeoutTag) {
549-
return;
550-
}
551554

552555
STORAGE_CHECK_PRECONDITION(DrainSource == EDrainSource::External);
553556

557+
Y_UNUSED(ev);
558+
554559
LOG_WARN(
555560
ctx,
556561
TBlockStoreComponents::VOLUME,
@@ -709,31 +714,47 @@ void TCheckpointActor<TMethod>::HandlePoisonPill(
709714
////////////////////////////////////////////////////////////////////////////////
710715

711716
template <typename TMethod>
712-
void TCheckpointActor<TMethod>::HandleReleaseShadowDisk(
717+
void TCheckpointActor<TMethod>::HandleReleaseDiskResponse(
713718
const TEvDiskRegistry::TEvReleaseDiskResponse::TPtr& ev,
714719
const TActorContext& ctx)
715720
{
716-
auto& record = ev->Get()->Record;
721+
const auto& record = ev->Get()->Record;
722+
723+
const auto releaseSessionKind =
724+
static_cast<EReleaseSessionKind>(ev->Cookie);
717725

718726
if (record.HasError()) {
719727
if (GetErrorKind(record.GetError()) == EErrorKind::ErrorRetriable) {
720-
ctx.Schedule(
728+
ctx.ExecutorThread.Schedule(
721729
ReleaseShadowDiskRetryTimeout,
722-
new TEvents::TEvWakeup(ev->Cookie));
730+
new IEventHandle(
731+
MakeDiskRegistryProxyServiceId(),
732+
TBase::SelfId(),
733+
MakeReleaseDiskRequest(releaseSessionKind).release(),
734+
0, // flags
735+
ev->Cookie));
723736

724737
return;
725738
}
726739

727-
auto errorMsg = record.GetError().GetMessage().data();
728740
ReportReleaseShadowDiskError(
729741
TStringBuilder()
730-
<< "Error: " << errorMsg
731-
<< " while trying to release shadow disk with id : "
732-
<< ShadowDiskId);
742+
<< "Could not release shadow disk " << ShadowDiskId.Quote()
743+
<< " Error: " << FormatError(record.GetError()));
733744
}
734745

735-
if (!OneReleaseShadowDiskRequestFinished) {
736-
OneReleaseShadowDiskRequestFinished = true;
746+
switch (releaseSessionKind) {
747+
case EReleaseSessionKind::AnyReader:
748+
ReleaseShadowDiskRequestForAnyReaderFinished = true;
749+
break;
750+
case EReleaseSessionKind::AnyWriter:
751+
ReleaseShadowDiskRequestForAnyWriterFinished = true;
752+
break;
753+
}
754+
755+
if (!ReleaseShadowDiskRequestForAnyReaderFinished ||
756+
!ReleaseShadowDiskRequestForAnyWriterFinished)
757+
{
737758
return;
738759
}
739760

@@ -746,35 +767,6 @@ void TCheckpointActor<TMethod>::HandleReleaseShadowDisk(
746767
}
747768
}
748769

749-
template <typename TMethod>
750-
void TCheckpointActor<TMethod>::HandleReleaseShadowDiskRetry(
751-
const TEvents::TEvWakeup::TPtr& ev,
752-
const TActorContext& ctx)
753-
{
754-
auto tag = ev->Get()->Tag;
755-
756-
if (tag != ReleaseForAnyReader && tag != ReleaseForAnyWriter) {
757-
return;
758-
}
759-
760-
TString clientId;
761-
762-
switch (tag) {
763-
case ReleaseForAnyReader:
764-
clientId = AnyReaderClientId;
765-
break;
766-
case ReleaseForAnyWriter:
767-
clientId = AnyWriterClientId;
768-
break;
769-
}
770-
771-
NCloud::Send(
772-
ctx,
773-
MakeDiskRegistryProxyServiceId(),
774-
MakeReleaseDiskRequest(clientId),
775-
tag);
776-
}
777-
778770
///////////////////////////////////////////////////////////////////////////////
779771

780772
template <typename TMethod>
@@ -848,8 +840,7 @@ template <typename TMethod>
848840
STFUNC(TCheckpointActor<TMethod>::StateReleaseShadowDisk)
849841
{
850842
switch (ev->GetTypeRewrite()) {
851-
HFunc(TEvDiskRegistry::TEvReleaseDiskResponse, HandleReleaseShadowDisk);
852-
HFunc(TEvents::TEvWakeup, HandleReleaseShadowDiskRetry);
843+
HFunc(TEvDiskRegistry::TEvReleaseDiskResponse, HandleReleaseDiskResponse);
853844
HFunc(TEvents::TEvPoisonPill, HandlePoisonPill);
854845

855846
default:

‎cloud/blockstore/libs/storage/volume/volume_ut_checkpoint.cpp

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5294,11 +5294,12 @@ Y_UNIT_TEST_SUITE(TVolumeCheckpointTest)
52945294

52955295
volume.CreateCheckpoint("c1");
52965296

5297-
constexpr auto ReleaseForAnyWriter = 1;
5298-
constexpr auto ReleaseForAnyReader = 2;
5297+
constexpr auto ReleaseForAnyReader = 1;
5298+
constexpr auto ReleaseForAnyWriter = 2;
52995299

5300-
bool seenReleaseShadowDiskForAnyWriter = false;
53015300
bool seenReleaseShadowDiskForAnyReader = false;
5301+
bool seenReleaseShadowDiskForAnyWriter = false;
5302+
53025303
auto observerReleaseShadowDiskRequest =
53035304
[&](TEvDiskRegistry::TEvReleaseDiskRequest::TPtr& event)
53045305
{
@@ -5343,11 +5344,12 @@ Y_UNIT_TEST_SUITE(TVolumeCheckpointTest)
53435344
volume.CreateCheckpoint("c1");
53445345
volume.CreateCheckpoint("c2");
53455346

5346-
constexpr auto ReleaseForAnyWriter = 1;
5347-
constexpr auto ReleaseForAnyReader = 2;
5347+
constexpr auto ReleaseForAnyReader = 1;
5348+
constexpr auto ReleaseForAnyWriter = 2;
53485349

5349-
bool seenReleaseShadowDiskForAnyWriter = false;
53505350
bool seenReleaseShadowDiskForAnyReader = false;
5351+
bool seenReleaseShadowDiskForAnyWriter = false;
5352+
53515353
auto observerReleaseShadowDiskRequest =
53525354
[&](TEvDiskRegistry::TEvReleaseDiskRequest::TPtr& event)
53535355
{
@@ -5399,44 +5401,56 @@ Y_UNIT_TEST_SUITE(TVolumeCheckpointTest)
53995401
NCloud::NProto::STORAGE_MEDIA_SSD_NONREPLICATED,
54005402
32768);
54015403

5402-
constexpr auto ReleaseForAnyWriter = 1;
5403-
constexpr auto ReleaseForAnyReader = 2;
5404+
constexpr auto ReleaseForAnyReader = 1;
5405+
constexpr auto ReleaseForAnyWriter = 2;
5406+
5407+
bool seenReleaseShadowDiskForAnyReader = false;
5408+
bool seenReleaseShadowDiskForAnyWriter = false;
5409+
5410+
bool seenReleaseShadowDiskRetryForAnyReader = false;
5411+
bool seenReleaseShadowDiskRetryForAnyWriter = false;
5412+
5413+
bool returnErrorForAnyReader = false;
5414+
bool returnErrorForAnyWriter = false;
54045415

5405-
ui32 numberSeenReleaseShadowDiskForAnyWriter = 0;
5406-
ui32 numberSeenReleaseShadowDiskForAnyReader = 0;
54075416
auto observerReleaseShadowDiskRequest =
54085417
[&](TEvDiskRegistry::TEvReleaseDiskRequest::TPtr& event)
54095418
{
54105419
if (event->Cookie == ReleaseForAnyReader) {
5411-
++numberSeenReleaseShadowDiskForAnyReader;
5420+
if (!returnErrorForAnyReader) {
5421+
seenReleaseShadowDiskForAnyReader = true;
5422+
} else {
5423+
seenReleaseShadowDiskRetryForAnyReader = true;
5424+
}
54125425
}
54135426

54145427
if (event->Cookie == ReleaseForAnyWriter) {
5415-
++numberSeenReleaseShadowDiskForAnyWriter;
5428+
if (!returnErrorForAnyWriter) {
5429+
seenReleaseShadowDiskForAnyWriter = true;
5430+
} else {
5431+
seenReleaseShadowDiskRetryForAnyWriter = true;
5432+
}
54165433
}
54175434
};
54185435
auto handleObserverReleaseRequest =
54195436
runtime->AddObserver<TEvDiskRegistry::TEvReleaseDiskRequest>(
54205437
observerReleaseShadowDiskRequest);
54215438

5422-
bool returnErrorForAnyWriter = false;
5423-
bool returnErrorForAnyReader = false;
5424-
54255439
auto releaseDiskResponseFilter =
54265440
[&](TEvDiskRegistry::TEvReleaseDiskResponse::TPtr& event)
54275441
{
54285442
// Simulate response with error.
5429-
if (event->Cookie == ReleaseForAnyWriter &&
5430-
!returnErrorForAnyWriter)
5443+
if (event->Cookie == ReleaseForAnyReader &&
5444+
!returnErrorForAnyReader)
54315445
{
5432-
returnErrorForAnyWriter = true;
5446+
returnErrorForAnyReader = true;
54335447
event->Get()->Record.MutableError()->SetCode(E_REJECTED);
54345448
}
54355449

5436-
if (event->Cookie == ReleaseForAnyReader &&
5437-
!returnErrorForAnyReader)
5450+
if (event->Cookie == ReleaseForAnyWriter &&
5451+
!returnErrorForAnyWriter)
54385452
{
5439-
returnErrorForAnyReader = true;
5453+
returnErrorForAnyWriter = true;
54405454
event->Get()->Record.MutableError()->SetCode(E_REJECTED);
54415455
}
54425456
};
@@ -5456,10 +5470,13 @@ Y_UNIT_TEST_SUITE(TVolumeCheckpointTest)
54565470
UNIT_ASSERT(returnErrorForAnyReader);
54575471
UNIT_ASSERT(returnErrorForAnyWriter);
54585472

5459-
// check that we saw two release request(one original and one retry)
5460-
// for both type client: any-writer and any-reader
5461-
UNIT_ASSERT_EQUAL(numberSeenReleaseShadowDiskForAnyReader, 2);
5462-
UNIT_ASSERT_EQUAL(numberSeenReleaseShadowDiskForAnyWriter, 2);
5473+
// check that we saw original request for any-reader and any-writer
5474+
UNIT_ASSERT(seenReleaseShadowDiskForAnyReader);
5475+
UNIT_ASSERT(seenReleaseShadowDiskForAnyWriter);
5476+
5477+
// check that we saw retry request for any-reader and any-writer
5478+
UNIT_ASSERT(seenReleaseShadowDiskRetryForAnyReader);
5479+
UNIT_ASSERT(seenReleaseShadowDiskRetryForAnyWriter);
54635480
}
54645481
}
54655482

0 commit comments

Comments
 (0)