From 15cb02273d14fa187e85ae1715e4fa4c0e20a447 Mon Sep 17 00:00:00 2001 From: gayurgin Date: Wed, 31 Jan 2024 21:22:58 +0300 Subject: [PATCH] NBS-4665, NBS-4262: checkpoints: small fixes --- .../libs/storage/volume/model/checkpoint.cpp | 15 +++++++--- .../libs/storage/volume/model/checkpoint.h | 5 ++-- .../storage/volume/model/checkpoint_ut.cpp | 13 ++++----- .../libs/storage/volume/volume_actor.h | 2 +- .../volume/volume_actor_checkpoint.cpp | 29 ++++++++----------- 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp b/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp index 8feea8c10ba..813d76033f2 100644 --- a/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp +++ b/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp @@ -297,14 +297,21 @@ void TCheckpointStore::Apply(const TCheckpointRequest& checkpointRequest) } } -TSet& TCheckpointStore::GetCheckpoitsWithSavedRequest() +bool TCheckpointStore::HasSavedRequest(const TString& checkpointId) const { - return CheckpointsWithSavedRequest; + return CheckpointsWithSavedRequest.contains(checkpointId); +} + +void TCheckpointStore::MarkHasSavedRequest(TString checkpointId) +{ + Y_DEBUG_ABORT_UNLESS(!CheckpointsWithSavedRequest.contains(checkpointId)); + CheckpointsWithSavedRequest.insert(checkpointId); } -const TSet& TCheckpointStore::GetCheckpoitsWithSavedRequest() const +void TCheckpointStore::MarkHasNoSavedRequest(TString checkpointId) { - return CheckpointsWithSavedRequest; + Y_DEBUG_ABORT_UNLESS(CheckpointsWithSavedRequest.contains(checkpointId)); + CheckpointsWithSavedRequest.erase(checkpointId); } //////////////////////////////////////////////////////////////////////////////// diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint.h b/cloud/blockstore/libs/storage/volume/model/checkpoint.h index d3123043154..28f84981060 100644 --- a/cloud/blockstore/libs/storage/volume/model/checkpoint.h +++ b/cloud/blockstore/libs/storage/volume/model/checkpoint.h @@ -151,8 +151,9 @@ class TCheckpointStore [[nodiscard]] TVector GetCheckpointRequests() const; - TSet& GetCheckpoitsWithSavedRequest(); - const TSet& GetCheckpoitsWithSavedRequest() const; + bool HasSavedRequest(const TString& checkpointId) const; + void MarkHasSavedRequest(TString checkpointId); + void MarkHasNoSavedRequest(TString checkpointId); private: [[nodiscard]] TCheckpointRequest& GetRequest(ui64 requestId); diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp b/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp index 386b47df936..c7ec6f38591 100644 --- a/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp +++ b/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp @@ -191,13 +191,12 @@ Y_UNIT_TEST_SUITE(TCheckpointStore) UNIT_ASSERT_VALUES_EQUAL(true, store.IsCheckpointDeleted("checkpoint-4")); UNIT_ASSERT_VALUES_EQUAL(false, store.IsCheckpointDeleted("checkpoint-5")); - const auto& checkpoitsWithSavedRequest = store.GetCheckpoitsWithSavedRequest(); - UNIT_ASSERT_VALUES_EQUAL(false, checkpoitsWithSavedRequest.contains("checkpoint-1")); - UNIT_ASSERT_VALUES_EQUAL(false, checkpoitsWithSavedRequest.contains("checkpoint-2")); - UNIT_ASSERT_VALUES_EQUAL(true, checkpoitsWithSavedRequest.contains("checkpoint-3")); - UNIT_ASSERT_VALUES_EQUAL(false, checkpoitsWithSavedRequest.contains("checkpoint-4")); - UNIT_ASSERT_VALUES_EQUAL(false, checkpoitsWithSavedRequest.contains("checkpoint-5")); - UNIT_ASSERT_VALUES_EQUAL(false, checkpoitsWithSavedRequest.contains("checkpoint-6")); + UNIT_ASSERT_VALUES_EQUAL(false, store.HasSavedRequest("checkpoint-1")); + UNIT_ASSERT_VALUES_EQUAL(false, store.HasSavedRequest("checkpoint-2")); + UNIT_ASSERT_VALUES_EQUAL(true, store.HasSavedRequest("checkpoint-3")); + UNIT_ASSERT_VALUES_EQUAL(false, store.HasSavedRequest("checkpoint-4")); + UNIT_ASSERT_VALUES_EQUAL(false, store.HasSavedRequest("checkpoint-5")); + UNIT_ASSERT_VALUES_EQUAL(false, store.HasSavedRequest("checkpoint-6")); } Y_UNIT_TEST(CreateFail) diff --git a/cloud/blockstore/libs/storage/volume/volume_actor.h b/cloud/blockstore/libs/storage/volume/volume_actor.h index ffb57052bd5..2ebe192fcb3 100644 --- a/cloud/blockstore/libs/storage/volume/volume_actor.h +++ b/cloud/blockstore/libs/storage/volume/volume_actor.h @@ -643,7 +643,7 @@ class TVolumeActor final const TCheckpointRequest& request, const TCheckpointRequestInfo& info); - bool TryReplyToCheckpointRequestWithoutSaving( + bool TryReplyToCheckpointRequest( const NActors::TActorContext& ctx, const TCheckpointRequest& request, const TCheckpointRequestInfo& info); diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp index 9ef0b3c0b36..d0fdad7fa46 100644 --- a/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp +++ b/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp @@ -954,7 +954,7 @@ void TVolumeActor::AddCheckpointRequest( const TCheckpointRequest& request, const TCheckpointRequestInfo& info) { - if (TryReplyToCheckpointRequestWithoutSaving(ctx, request, info)) { + if (TryReplyToCheckpointRequest(ctx, request, info)) { return; } @@ -971,8 +971,7 @@ void TVolumeActor::AddCheckpointRequest( return; } - if (!State->GetCheckpointStore().GetCheckpoitsWithSavedRequest().contains( - request.CheckpointId)) { + if (!State->GetCheckpointStore().HasSavedRequest(request.CheckpointId)) { SaveCheckpointRequest(ctx, request, info); return; } @@ -989,15 +988,15 @@ void TVolumeActor::AddCheckpointRequest( info}); } -bool TVolumeActor::TryReplyToCheckpointRequestWithoutSaving( +bool TVolumeActor::TryReplyToCheckpointRequest( const TActorContext& ctx, const TCheckpointRequest& request, const TCheckpointRequestInfo& info) { - std::optional error = + std::optional validationResult = ValidateCheckpointRequest(ctx, request); - if (!error) { + if (!validationResult) { return false; } @@ -1006,15 +1005,15 @@ bool TVolumeActor::TryReplyToCheckpointRequestWithoutSaving( switch (request.ReqType) { case ECheckpointRequestType::Create: case ECheckpointRequestType::CreateWithoutData: { - response = std::make_unique(*error); + response = std::make_unique(*validationResult); break; } case ECheckpointRequestType::Delete: { - response = std::make_unique(*error); + response = std::make_unique(*validationResult); break; } case ECheckpointRequestType::DeleteData: { - response = std::make_unique(*error); + response = std::make_unique(*validationResult); break; } } @@ -1041,9 +1040,8 @@ void TVolumeActor::ProcessPostponedCheckpointRequests( const auto& request = State->GetCheckpointStore().GetRequestById( ponstponedRequestInfo->RequestId); - if (!TryReplyToCheckpointRequestWithoutSaving(ctx, request, ponstponedRequestInfo->Info)) { - if (!State->GetCheckpointStore().GetCheckpoitsWithSavedRequest().contains( - request.CheckpointId)) { + if (!TryReplyToCheckpointRequest(ctx, request, ponstponedRequestInfo->Info)) { + if (!State->GetCheckpointStore().HasSavedRequest(request.CheckpointId)) { PostponedCheckpointRequests.RemovePostponedRequest(checkpointId); SaveCheckpointRequest(ctx, request, ponstponedRequestInfo->Info); } @@ -1068,8 +1066,7 @@ void TVolumeActor::SaveCheckpointRequest( const TCheckpointRequest& request, const TCheckpointRequestInfo& info) { - State->GetCheckpointStore().GetCheckpoitsWithSavedRequest().insert( - request.CheckpointId); + State->GetCheckpointStore().MarkHasSavedRequest(request.CheckpointId); AddTransaction(*info.RequestInfo); @@ -1394,9 +1391,7 @@ void TVolumeActor::ExecuteUpdateCheckpointRequest( auto checkpointRequestCopy = State->GetCheckpointStore().GetRequestById(args.RequestId); - auto& checkpointsWithSavedRequest = - State->GetCheckpointStore().GetCheckpoitsWithSavedRequest(); - checkpointsWithSavedRequest.erase(checkpointRequestCopy.CheckpointId); + State->GetCheckpointStore().MarkHasNoSavedRequest(checkpointRequestCopy.CheckpointId); TVolumeDatabase db(tx.DB); db.UpdateCheckpointRequest(