fix usage of data preparing in checkpoint request validation

ydb-platform · Jan 25, 2024 · 17b03a1 · 17b03a1
1 parent 86b92a4
commit 17b03a1
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp b/cloud/blockstore/libs/storage/volume/model/checkpoint.cpp
@@ -106,6 +106,15 @@ bool TCheckpointStore::DoesCheckpointHaveData(const TString& checkpointId) const
     return false;
 }
 
+bool TCheckpointStore::IsCheckpointDataPreparingOrPresent(const TString& checkpointId) const
+{
+    if (const auto* data = ActiveCheckpoints.FindPtr(checkpointId)) {
+        return data->Data == ECheckpointData::DataPreparing
+            || data->Data == ECheckpointData::DataPresent;
+    }
+    return false;
+}
+
 bool TCheckpointStore::HasRequestToExecute(ui64* requestId) const
 {
     Y_DEBUG_ABORT_UNLESS(requestId);
@@ -328,7 +337,8 @@ ECheckpointRequestValidityStatus TCheckpointStore::ValidateCheckpointRequest(
     }
 
     const bool checkpointExists = actualCheckpointType.has_value();
-    const bool checkpointHasData = checkpointExists && DoesCheckpointHaveData(checkpointId);
+    const bool checkpointDataPreparingOrPresent = checkpointExists
+        && IsCheckpointDataPreparingOrPresent(checkpointId);
     const bool checkpointDeleted = IsCheckpointDeleted(checkpointId);
 
     if (!checkpointExists && !checkpointDeleted) {
@@ -347,7 +357,7 @@ ECheckpointRequestValidityStatus TCheckpointStore::ValidateCheckpointRequest(
                 return ECheckpointRequestValidityStatus::Already;
             }
         }
-    } else if (checkpointExists && checkpointHasData) {
+    } else if (checkpointExists && checkpointDataPreparingOrPresent) {
         // Checkpoint exists and has data.
         switch (requestType) {
             case ECheckpointRequestType::Create: {
@@ -365,7 +375,7 @@ ECheckpointRequestValidityStatus TCheckpointStore::ValidateCheckpointRequest(
                 return ECheckpointRequestValidityStatus::Invalid;
             }
         }
-    } else if (checkpointExists && !checkpointHasData) {
+    } else if (checkpointExists && !checkpointDataPreparingOrPresent) {
         // Checkpoint exists and has no data.
         switch (requestType) {
             case ECheckpointRequestType::Create: {

diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint.h b/cloud/blockstore/libs/storage/volume/model/checkpoint.h
@@ -133,6 +133,8 @@ class TCheckpointStore
     [[nodiscard]] bool DoesCheckpointBlockingWritesExist() const;
     [[nodiscard]] bool DoesCheckpointHaveData(
         const TString& checkpointId) const;
+    [[nodiscard]] bool IsCheckpointDataPreparingOrPresent(
+        const TString& checkpointId) const;
 
     [[nodiscard]] std::optional<ECheckpointType> GetCheckpointType(
         const TString& checkpointId) const;

diff --git a/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp b/cloud/blockstore/libs/storage/volume/model/checkpoint_ut.cpp
@@ -126,6 +126,15 @@ Y_UNIT_TEST_SUITE(TCheckpointStore)
                 ECheckpointRequestState::Completed,
                 ECheckpointType::Normal,
                 ""},
+
+            TCheckpointRequest{
+                13,
+                "checkpoint-6",
+                TInstant::Now(),
+                ECheckpointRequestType::Create,
+                ECheckpointRequestState::Completed,
+                ECheckpointType::Normal,
+                "shadow_disk"},
         };
         TCheckpointStore store(
             TVector<TCheckpointRequest>{
@@ -142,10 +151,11 @@ Y_UNIT_TEST_SUITE(TCheckpointStore)
         UNIT_ASSERT_VALUES_EQUAL(10, requestId);
 
         auto checkpoints = store.GetActiveCheckpoints();
-        UNIT_ASSERT_VALUES_EQUAL(3, checkpoints.size());
+        UNIT_ASSERT_VALUES_EQUAL(4, checkpoints.size());
         UNIT_ASSERT_VALUES_EQUAL(true, checkpoints.contains("checkpoint-1"));
         UNIT_ASSERT_VALUES_EQUAL(true, checkpoints.contains("checkpoint-2"));
         UNIT_ASSERT_VALUES_EQUAL(true, checkpoints.contains("checkpoint-5"));
+        UNIT_ASSERT_VALUES_EQUAL(true, checkpoints.contains("checkpoint-6"));
 
         UNIT_ASSERT_VALUES_EQUAL(
             true,
@@ -156,6 +166,24 @@ Y_UNIT_TEST_SUITE(TCheckpointStore)
         UNIT_ASSERT_VALUES_EQUAL(
             false,
             store.DoesCheckpointHaveData("checkpoint-5"));
+        UNIT_ASSERT_VALUES_EQUAL(
+            false,
+            store.DoesCheckpointHaveData("checkpoint-6"));
+
+        UNIT_ASSERT_VALUES_EQUAL(1, store.GetCheckpointsWithData().size());
+
+        UNIT_ASSERT_VALUES_EQUAL(
+            true,
+            store.IsCheckpointDataPreparingOrPresent("checkpoint-2"));
+        UNIT_ASSERT_VALUES_EQUAL(
+            false,
+            store.IsCheckpointDataPreparingOrPresent("checkpoint-1"));
+        UNIT_ASSERT_VALUES_EQUAL(
+            false,
+            store.IsCheckpointDataPreparingOrPresent("checkpoint-5"));
+        UNIT_ASSERT_VALUES_EQUAL(
+            true,
+            store.IsCheckpointDataPreparingOrPresent("checkpoint-6"));
 
         UNIT_ASSERT_VALUES_EQUAL(false, store.IsCheckpointDeleted("checkpoint-1"));
         UNIT_ASSERT_VALUES_EQUAL(false, store.IsCheckpointDeleted("checkpoint-2"));
@@ -169,6 +197,7 @@ Y_UNIT_TEST_SUITE(TCheckpointStore)
         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"));
     }
 
     Y_UNIT_TEST(CreateFail)