Fix wrong checkpoint info after restart (#7801)

close #7799
pingcap · Jul 13, 2023 · a4f999a · a4f999a
1 parent c04008c
commit a4f999a
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 15 deletions.
diff --git a/dbms/src/Storages/Page/V3/PageDirectory.cpp b/dbms/src/Storages/Page/V3/PageDirectory.cpp
@@ -529,9 +529,10 @@ void VersionedPageEntries<Trait>::copyCheckpointInfoFromEdit(const typename Page
     // (the checkpoint info) each page's data was dumped.
     // In this case, there is a living snapshot protecting the data.
 
-    // Pre-check: All ENTRY edit record must contain checkpoint info for copying.
     RUNTIME_CHECK(edit.type == EditRecordType::VAR_ENTRY);
-    RUNTIME_CHECK(edit.entry.checkpoint_info.has_value());
+    // The checkpoint_info from `edit` could be empty when we upload the manifest without any page data
+    if (!edit.entry.checkpoint_info.has_value())
+        return;
 
     auto page_lock = acquireLock();
 
@@ -1854,14 +1855,6 @@ size_t PageDirectory<Trait>::copyCheckpointInfoFromEdit(const PageEntriesEdit &
     if (records.empty())
         return num_copied;
 
-    // Pre-check: All ENTRY edit record must contain checkpoint info.
-    // We do the pre-check before copying any remote info to avoid partial completion.
-    for (const auto & rec : records)
-    {
-        if (rec.type == EditRecordType::VAR_ENTRY)
-            RUNTIME_CHECK_MSG(rec.entry.checkpoint_info.has_value(), "try to copy checkpoint from an edit with invalid record: {}", rec);
-    }
-
     for (const auto & rec : records)
     {
         // Only VAR_ENTRY need update checkpoint info.

diff --git a/dbms/src/Storages/Page/V3/PageEntriesEdit.cpp b/dbms/src/Storages/Page/V3/PageEntriesEdit.cpp
@@ -64,11 +64,16 @@ typename PageEntriesEdit<UniversalPageId>::EditRecord PageEntriesEdit<UniversalP
     rec.being_ref_count = 1;
     if (rec.type == EditRecordType::VAR_ENTRY)
     {
-        rec.entry.checkpoint_info = OptionalCheckpointInfo{
-            .data_location = CheckpointLocation::fromProto(proto_edit.entry_location(), strings_map),
-            .is_valid = true,
-            .is_local_data_reclaimed = true,
-        };
+        // uploading page data may be disabled
+        auto checkpoint_loc = CheckpointLocation::fromProto(proto_edit.entry_location(), strings_map);
+        if (checkpoint_loc.isValid())
+        {
+            rec.entry.checkpoint_info = OptionalCheckpointInfo{
+                .data_location = std::move(checkpoint_loc),
+                .is_valid = true,
+                .is_local_data_reclaimed = true,
+            };
+        }
         rec.entry.size = proto_edit.entry_size();
         rec.entry.checksum = proto_edit.entry_checksum();
         rec.entry.tag = proto_edit.entry_tag();

diff --git a/dbms/src/Storages/Page/V3/PageEntryCheckpointInfo.h b/dbms/src/Storages/Page/V3/PageEntryCheckpointInfo.h
@@ -47,6 +47,8 @@ struct CheckpointLocation
         const CheckpointProto::EntryDataLocation & proto_rec,
         CheckpointProto::StringsInternMap & strings_map);
 
+    bool isValid() const { return !data_file_id->empty(); }
+
     std::string toDebugString() const
     {
         return fmt::format("{{data_file_id: {}, offset_in_file: {}, size_in_file: {}}}", *data_file_id, offset_in_file, size_in_file);