From b2766fdb547f05d2af2d7a730327001c6371d6e2 Mon Sep 17 00:00:00 2001 From: David Rogers Date: Fri, 26 Jan 2024 23:56:28 +0000 Subject: [PATCH] blob_store: Resumable BlobWriter Add support for resumable BlobWriter. Do some cleanup of reader open logic to improve correctness. Change-Id: I6dedd9e69fd21fb241e03c9dcf16356d69f1c172 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/185056 Pigweed-Auto-Submit: David Rogers Reviewed-by: Armando Montanez Presubmit-Verified: CQ Bot Account Commit-Queue: Auto-Submit Reviewed-by: Jordan Brauer --- pw_blob_store/blob_store.cc | 140 +++++++-- pw_blob_store/blob_store_test.cc | 267 +++++++++++++++--- pw_blob_store/docs.rst | 27 +- .../public/pw_blob_store/blob_store.h | 115 +++++++- 4 files changed, 475 insertions(+), 74 deletions(-) diff --git a/pw_blob_store/blob_store.cc b/pw_blob_store/blob_store.cc index 38305d664b..63ed9abc93 100644 --- a/pw_blob_store/blob_store.cc +++ b/pw_blob_store/blob_store.cc @@ -90,7 +90,10 @@ Status BlobStore::LoadMetadata() { metadata.v1_metadata.checksum) .ok()) { PW_LOG_ERROR("BlobStore init - Invalidating blob with invalid checksum"); - Invalidate().IgnoreError(); // TODO: b/242598609 - Handle Status properly + + // Invalidate return status can be safely be ignored, are already about to + // return the correct error status. + Invalidate().IgnoreError(); return Status::DataLoss(); } @@ -119,12 +122,64 @@ Status BlobStore::OpenWrite() { writer_open_ = true; - // Clear any existing contents. - Invalidate().IgnoreError(); // TODO: b/242598609 - Handle Status properly + // Clear any existing contents. Invalidate return status can be safely be + // ignored, only KVS::Delete can result in an error and that KVS entry will + // overwriten on write Close. + Invalidate().IgnoreError(); return OkStatus(); } +StatusWithSize BlobStore::ResumeWrite() { + if (!initialized_) { + return StatusWithSize::FailedPrecondition(); + } + + // Writer can only be opened if there are no other writer or readers already + // open and also if there is no completed blob (opening completed blob to + // append is not currently supported). + if (writer_open_ || readers_open_ != 0 || valid_data_) { + return StatusWithSize::Unavailable(); + } + + // Clear any existing blob state or KVS key, to provide a consistent starting + // point for resume. + // + // Invalidate return status can be safely be ignored, only KVS::Delete can + // result in an error and that KVS entry will overwriten on write Close. + Invalidate().IgnoreError(); + StatusWithSize written_sws = partition_.EndOfWrittenData(); + PW_TRY_WITH_SIZE(StatusWithSize(written_sws.status(), 0)); + + // Round down to the number of fully written sectors. + size_t written_sectors = written_sws.size() / partition_.sector_size_bytes(); + + // Drop the last full written sector, to back things up in case the last bit + // written data was corrupted. + written_sectors = written_sectors == 0 ? 0 : written_sectors - 1; + + size_t written_bytes_on_resume = + written_sectors * partition_.sector_size_bytes(); + + // Erase the 2 sectors after the kept written sectors. This is a full sector + // and any possible partitial sector after the kept data. + size_t sectors_to_erase = + std::min(2, (partition_.sector_count() - written_sectors)); + PW_TRY_WITH_SIZE(partition_.Erase(written_bytes_on_resume, sectors_to_erase)); + + PW_TRY_WITH_SIZE(CalculateChecksumFromFlash(written_bytes_on_resume, false)); + + flash_address_ = written_bytes_on_resume; + write_address_ = written_bytes_on_resume; + valid_data_ = true; + writer_open_ = true; + + PW_LOG_DEBUG("Blob writer open for resume with %zu bytes", + written_bytes_on_resume); + + return StatusWithSize(OkStatus(), written_bytes_on_resume); +} + StatusWithSize BlobStore::GetFileName(span dest) const { if (!initialized_) { return StatusWithSize(Status::FailedPrecondition(), 0); @@ -453,7 +508,9 @@ Status BlobStore::Erase() { // If any writes have been performed, reset the state. if (flash_address_ != 0) { - Invalidate().IgnoreError(); // TODO: b/242598609 - Handle Status properly + // Invalidate return status can be safely be ignored, only KVS::Delete can + // result in an error and that KVS entry will overwriten on write Close. + Invalidate().IgnoreError(); } PW_TRY(partition_.Erase()); @@ -500,7 +557,7 @@ Status BlobStore::ValidateChecksum(size_t blob_size_bytes, PW_LOG_DEBUG("Validate checksum of 0x%08x in flash for blob of %u bytes", static_cast(expected), static_cast(blob_size_bytes)); - PW_TRY(CalculateChecksumFromFlash(blob_size_bytes)); + PW_TRY(CalculateChecksumFromFlash(blob_size_bytes, true)); Status status = checksum_algo_->Verify(as_bytes(span(&expected, 1))); PW_LOG_DEBUG(" checksum verify of %s", status.str()); @@ -508,7 +565,8 @@ Status BlobStore::ValidateChecksum(size_t blob_size_bytes, return status; } -Status BlobStore::CalculateChecksumFromFlash(size_t bytes_to_check) { +Status BlobStore::CalculateChecksumFromFlash(size_t bytes_to_check, + bool finish) { if (checksum_algo_ == nullptr) { return OkStatus(); } @@ -528,9 +586,11 @@ Status BlobStore::CalculateChecksumFromFlash(size_t bytes_to_check) { address += read_size; } - // Safe to ignore the return from Finish, checksum_algo_ keeps the state - // information that it needs. - checksum_algo_->Finish(); + if (finish) { + // Safe to ignore the return from Finish, checksum_algo_ keeps the state + // information that it needs. + checksum_algo_->Finish(); + } return OkStatus(); } @@ -555,6 +615,29 @@ Status BlobStore::BlobWriter::SetFileName(std::string_view file_name) { return OkStatus(); } +StatusWithSize BlobStore::BlobWriter::GetFileName(span dest) { + if (!open_) { + return StatusWithSize::FailedPrecondition(); + } + PW_DCHECK(store_.writer_open_); + + if (store_.file_name_length_ == 0) { + return StatusWithSize(Status::NotFound(), 0); + } + + const size_t file_name_length = + std::min(store_.file_name_length_, dest.size_bytes()); + + // Get the file name from the encode buffer, just past the BlobMetadataHeader + // struct. Do this instead of using store_.GetFileName() because the writter + // has not yet flushed the name ot KVS yet. + constexpr size_t kFileNameOffset = sizeof(BlobMetadataHeader); + const ByteSpan file_name_dest = metadata_buffer_.subspan(kFileNameOffset); + std::memcpy(dest.data(), file_name_dest.data(), file_name_length); + + return StatusWithSize(file_name_length); +} + Status BlobStore::BlobWriter::Open() { PW_DCHECK(!open_); PW_DCHECK_UINT_GE(metadata_buffer_.size_bytes(), @@ -567,6 +650,18 @@ Status BlobStore::BlobWriter::Open() { return status; } +StatusWithSize BlobStore::BlobWriter::Resume() { + PW_DCHECK(!open_); + PW_DCHECK_UINT_GE(metadata_buffer_.size_bytes(), + sizeof(internal::BlobMetadataHeader)); + + const StatusWithSize sws = store_.ResumeWrite(); + if (sws.ok()) { + open_ = true; + } + return sws; +} + // Validates and commits BlobStore metadata to KVS. // // 1. Finalize checksum calculation. @@ -680,6 +775,18 @@ Status BlobStore::BlobWriter::Close() { return OkStatus(); } +Status BlobStore::BlobWriter::Abandon() { + if (!open_) { + return Status::FailedPrecondition(); + } + + store_.valid_data_ = false; + open_ = false; + store_.writer_open_ = false; + + return OkStatus(); +} + size_t BlobStore::BlobReader::ConservativeLimit(LimitType limit) const { if (open_ && limit == LimitType::kRead) { return store_.ReadableDataBytes() - offset_; @@ -690,16 +797,17 @@ size_t BlobStore::BlobReader::ConservativeLimit(LimitType limit) const { Status BlobStore::BlobReader::Open(size_t offset) { PW_DCHECK(!open_); PW_TRY(store_.Init()); - if (!store_.HasData()) { - return Status::FailedPrecondition(); - } - if (offset >= store_.ReadableDataBytes()) { - return Status::InvalidArgument(); - } - offset_ = offset; Status status = store_.OpenRead(); if (status.ok()) { + if (offset >= store_.ReadableDataBytes()) { + PW_LOG_ERROR( + "Blob reader unable open with offset greater than valid data"); + store_.CloseRead().IgnoreError(); + return Status::InvalidArgument(); + } + + offset_ = offset; open_ = true; } return status; diff --git a/pw_blob_store/blob_store_test.cc b/pw_blob_store/blob_store_test.cc index a87771aa33..eeea8db47b 100644 --- a/pw_blob_store/blob_store_test.cc +++ b/pw_blob_store/blob_store_test.cc @@ -83,14 +83,19 @@ class BlobStoreTest : public ::testing::Test { ASSERT_EQ(OkStatus(), writer.Write(write_data)); EXPECT_EQ(OkStatus(), writer.Close()); - // Use reader to check for valid data. + VerifyBlob(blob, write_size_bytes); + } + + void VerifyBlob(BlobStore& blob, size_t expected_data_size) { + EXPECT_TRUE(blob.HasData()); BlobStore::BlobReader reader(blob); ASSERT_EQ(OkStatus(), reader.Open()); + EXPECT_EQ(expected_data_size, reader.ConservativeReadLimit()); Result result = reader.GetMemoryMappedBlob(); ASSERT_TRUE(result.ok()); - EXPECT_EQ(write_size_bytes, result.value().size_bytes()); - VerifyFlash(result.value().first(write_size_bytes)); - VerifyFlash(flash_.buffer().first(write_size_bytes)); + EXPECT_EQ(expected_data_size, result.value().size_bytes()); + VerifyFlash(result.value().first(result.value().size_bytes())); + VerifyFlash(flash_.buffer().first(result.value().size_bytes())); EXPECT_EQ(OkStatus(), reader.Close()); } @@ -149,8 +154,8 @@ class BlobStoreTest : public ::testing::Test { } static constexpr size_t kFlashAlignment = PW_FLASH_TEST_ALIGNMENT; - static constexpr size_t kSectorSize = 2048; - static constexpr size_t kSectorCount = 2; + static constexpr size_t kSectorSize = 1024; + static constexpr size_t kSectorCount = 6; static constexpr size_t kBlobDataSize = (kSectorCount * kSectorSize); kvs::FakeFlashMemoryBuffer flash_; @@ -213,15 +218,7 @@ TEST_F(BlobStoreTest, OversizedWriteBuffer) { } EXPECT_EQ(OkStatus(), writer.Close()); - // Use reader to check for valid data. - BlobStore::BlobReader reader(blob); - ASSERT_EQ(OkStatus(), reader.Open()); - Result result = reader.GetMemoryMappedBlob(); - ASSERT_TRUE(result.ok()); - EXPECT_EQ(original_source.size_bytes(), result.value().size_bytes()); - VerifyFlash(result.value()); - VerifyFlash(flash_.buffer()); - EXPECT_EQ(OkStatus(), reader.Close()); + VerifyBlob(blob, original_source.size_bytes()); } TEST_F(BlobStoreTest, Reader_ConservativeLimits) { @@ -245,30 +242,39 @@ TEST_F(BlobStoreTest, IsOpen) { constexpr size_t kBufferSize = 256; BlobStoreBuffer blob( "Blob_open", partition_, nullptr, kvs::TestKvs(), kBufferSize); - EXPECT_EQ(OkStatus(), blob.Init()); BlobStore::DeferredWriterWithBuffer deferred_writer(blob); + BlobStore::BlobWriterWithBuffer regular_writer(blob); + BlobStore::BlobReader reader(blob); + EXPECT_EQ(Status::FailedPrecondition(), regular_writer.Open()); + EXPECT_EQ(Status::FailedPrecondition(), regular_writer.Resume().status()); + EXPECT_EQ(Status::FailedPrecondition(), reader.Open()); + + EXPECT_EQ(OkStatus(), blob.Init()); + EXPECT_EQ(false, deferred_writer.IsOpen()); EXPECT_EQ(OkStatus(), deferred_writer.Open()); EXPECT_EQ(true, deferred_writer.IsOpen()); + + EXPECT_EQ(Status::Unavailable(), regular_writer.Open()); + EXPECT_EQ(Status::Unavailable(), reader.Open()); + EXPECT_EQ(OkStatus(), deferred_writer.Close()); EXPECT_EQ(false, deferred_writer.IsOpen()); - BlobStore::BlobWriterWithBuffer writer(blob); - EXPECT_EQ(false, writer.IsOpen()); - EXPECT_EQ(OkStatus(), writer.Open()); - EXPECT_EQ(true, writer.IsOpen()); + EXPECT_EQ(false, regular_writer.IsOpen()); + EXPECT_EQ(OkStatus(), regular_writer.Open()); + EXPECT_EQ(true, regular_writer.IsOpen()); EXPECT_FALSE(blob.HasData()); // Need to write something, so the blob reader is able to open. std::array tmp_buffer = {}; - EXPECT_EQ(OkStatus(), writer.Write(tmp_buffer)); - EXPECT_EQ(OkStatus(), writer.Close()); - EXPECT_EQ(false, writer.IsOpen()); + EXPECT_EQ(OkStatus(), regular_writer.Write(tmp_buffer)); + EXPECT_EQ(OkStatus(), regular_writer.Close()); + EXPECT_EQ(false, regular_writer.IsOpen()); EXPECT_TRUE(blob.HasData()); - BlobStore::BlobReader reader(blob); EXPECT_EQ(false, reader.IsOpen()); ASSERT_EQ(OkStatus(), reader.Open()); EXPECT_EQ(true, reader.IsOpen()); @@ -320,15 +326,7 @@ TEST_F(BlobStoreTest, NoWriteBuffer_1Alignment) { EXPECT_EQ(write_data.size_bytes(), 0U); EXPECT_EQ(OkStatus(), writer.Close()); - // Use reader to check for valid data. - BlobStore::BlobReader reader(blob); - ASSERT_EQ(OkStatus(), reader.Open()); - Result result = reader.GetMemoryMappedBlob(); - ASSERT_TRUE(result.ok()); - EXPECT_EQ(original_source.size_bytes(), result.value().size_bytes()); - VerifyFlash(result.value()); - VerifyFlash(flash_.buffer()); - EXPECT_EQ(OkStatus(), reader.Close()); + VerifyBlob(blob, original_source.size_bytes()); } // Write to the blob using no write buffer size. Write operations must be @@ -376,15 +374,7 @@ TEST_F(BlobStoreTest, NoWriteBuffer_16Alignment) { } EXPECT_EQ(OkStatus(), writer.Close()); - // Use reader to check for valid data. - BlobStore::BlobReader reader(blob); - ASSERT_EQ(OkStatus(), reader.Open()); - Result result = reader.GetMemoryMappedBlob(); - ASSERT_TRUE(result.ok()); - EXPECT_EQ(original_source.size_bytes(), result.value().size_bytes()); - VerifyFlash(result.value()); - VerifyFlash(flash_.buffer()); - EXPECT_EQ(OkStatus(), reader.Close()); + VerifyBlob(blob, original_source.size_bytes()); } TEST_F(BlobStoreTest, FileName) { @@ -407,6 +397,16 @@ TEST_F(BlobStoreTest, FileName) { EXPECT_EQ(OkStatus(), writer.Open()); EXPECT_EQ(OkStatus(), writer.SetFileName(kFileName)); EXPECT_EQ(OkStatus(), writer.Write(tmp_buffer)); + + // Read back filename from the open writer. + memset(tmp_buffer.data(), 0, tmp_buffer.size()); + StatusWithSize sws = writer.GetFileName( + {reinterpret_cast(tmp_buffer.data()), tmp_buffer.size()}); + EXPECT_EQ(OkStatus(), sws.status()); + ASSERT_EQ(kFileName.size(), sws.size()); + EXPECT_EQ(memcmp(kFileName.data(), tmp_buffer.data(), kFileName.size()), 0); + // END of read back filename from the open writer. + EXPECT_EQ(OkStatus(), writer.Close()); EXPECT_EQ(OkStatus(), kvs::TestKvs().acquire()->Get(kBlobTitle, tmp_buffer).status()); @@ -464,6 +464,9 @@ TEST_F(BlobStoreTest, FileNameUndersizedSet) { InitSourceBufferToRandom(0x8675309); WriteTestBlock(); constexpr std::string_view kFileName("my_file_1.bin"); + std::array tmp_buffer = {}; + std::array zero_buffer = {}; + static_assert(kFileName.size() <= tmp_buffer.size()); kvs::ChecksumCrc16 checksum; constexpr size_t kBufferSize = 256; @@ -475,6 +478,18 @@ TEST_F(BlobStoreTest, FileNameUndersizedSet) { EXPECT_EQ(OkStatus(), writer.Open()); EXPECT_EQ(Status::ResourceExhausted(), writer.SetFileName(kFileName)); + + // Read back filename from the open writer. + memset(tmp_buffer.data(), 0, tmp_buffer.size()); + memset(zero_buffer.data(), 0, tmp_buffer.size()); + StatusWithSize sws = writer.GetFileName( + {reinterpret_cast(tmp_buffer.data()), tmp_buffer.size()}); + EXPECT_EQ(Status::NotFound(), sws.status()); + ASSERT_EQ(0U, sws.size()); + EXPECT_EQ(memcmp(zero_buffer.data(), tmp_buffer.data(), tmp_buffer.size()), + 0); + // END of read back filename from the open writer. + EXPECT_EQ(OkStatus(), writer.Close()); } @@ -499,6 +514,15 @@ TEST_F(BlobStoreTest, FileNameInvalidation) { EXPECT_EQ(OkStatus(), writer.Write(tmp_buffer)); EXPECT_EQ(OkStatus(), writer.Discard()); EXPECT_EQ(OkStatus(), writer.Write(tmp_buffer)); + + // Read back filename from the open writer. + { + StatusWithSize sws = writer.GetFileName( + {reinterpret_cast(tmp_buffer.data()), tmp_buffer.size()}); + EXPECT_EQ(Status::NotFound(), sws.status()); + ASSERT_EQ(0u, sws.size()); + } + EXPECT_EQ(OkStatus(), writer.Close()); // Check that the file name was discarded by Discard(). @@ -846,5 +870,164 @@ TEST_F(BlobStoreTest, MultipleWrites) { WriteTestBlock(); } +TEST_F(BlobStoreTest, ResumeEmptyAbandonBlob) { + InitSourceBufferToRandom(0x11309); + + kvs::ChecksumCrc16 checksum; + constexpr size_t kBufferSize = 256; + BlobStoreBuffer blob( + "TestBlobBlock", partition_, &checksum, kvs::TestKvs(), kBufferSize); + EXPECT_EQ(OkStatus(), blob.Init()); + + BlobStore::BlobWriterWithBuffer writer(blob); + BlobStore::BlobWriterWithBuffer second_writer(blob); + BlobStore::BlobReader reader(blob); + + EXPECT_EQ(OkStatus(), writer.Open()); + + EXPECT_EQ(OkStatus(), writer.Erase()); + EXPECT_EQ(OkStatus(), writer.Abandon()); + EXPECT_FALSE(blob.HasData()); + + StatusWithSize resume_sws = writer.Resume(); + EXPECT_EQ(OkStatus(), resume_sws.status()); + EXPECT_EQ(0U, resume_sws.size()); + + EXPECT_EQ(Status::Unavailable(), reader.Open()); + EXPECT_EQ(Status::Unavailable(), second_writer.Open()); + EXPECT_EQ(Status::Unavailable(), second_writer.Resume().status()); + + EXPECT_EQ(OkStatus(), writer.Abandon()); +} + +TEST_F(BlobStoreTest, ResumePartialAbandonBlob) { + InitSourceBufferToRandom(0x11309); + + kvs::ChecksumCrc16 checksum; + constexpr size_t kBufferSize = 16; + BlobStoreBuffer blob( + "TestBlobBlock", partition_, &checksum, kvs::TestKvs(), kBufferSize); + EXPECT_EQ(OkStatus(), blob.Init()); + + // The test fills 1/2 sector size less than the full blob size. Resume will + // backup 1.5 sectors for the resume point. Make sure the blob has enough + // sectors to do all that. + ASSERT_GE(kSectorCount, 3U); + + const size_t write_size_bytes = kBlobDataSize - (kSectorSize / 2); + ConstByteSpan write_data = span(source_buffer_).first(write_size_bytes); + + BlobStore::BlobWriterWithBuffer writer(blob); + EXPECT_EQ(OkStatus(), writer.Open()); + EXPECT_EQ(OkStatus(), writer.Write(write_data)); + EXPECT_EQ(OkStatus(), writer.Abandon()); + EXPECT_FALSE(blob.HasData()); + + StatusWithSize resume_sws = writer.Resume(); + EXPECT_EQ(OkStatus(), resume_sws.status()); + const size_t expected_resume_size = (kSectorCount - 2) * kSectorSize; + EXPECT_EQ(expected_resume_size, resume_sws.size()); + StatusWithSize eod_sws = partition_.EndOfWrittenData(); + EXPECT_EQ(OkStatus(), eod_sws.status()); + EXPECT_EQ(eod_sws.size(), resume_sws.size()); + VerifyFlash(flash_.buffer().first(resume_sws.size())); + + EXPECT_EQ(OkStatus(), + writer.Write(span(source_buffer_).subspan(resume_sws.size()))); + EXPECT_EQ(kBlobDataSize, writer.CurrentSizeBytes()); + EXPECT_EQ(OkStatus(), writer.Close()); + + VerifyBlob(blob, kBlobDataSize); +} + +TEST_F(BlobStoreTest, ResumePartialAfterRebootBlob) { + InitSourceBufferToRandom(0x11309); + + kvs::ChecksumCrc16 first_checksum; + constexpr size_t kBufferSize = 16; + BlobStoreBuffer first_blob("TestBlobBlock", + partition_, + &first_checksum, + kvs::TestKvs(), + kBufferSize); + EXPECT_EQ(OkStatus(), first_blob.Init()); + + // The test fills 1/2 sector size less than the full blob size. Resume will + // backup 1.5 sectors for the resume point. Make sure the blob has enough + // sectors to do all that. + ASSERT_GE(kSectorCount, 3U); + + const size_t write_size_bytes = kBlobDataSize - (kSectorSize / 2); + ConstByteSpan write_data = span(source_buffer_).first(write_size_bytes); + + // Write the data using + BlobStore::BlobWriterWithBuffer writer(first_blob); + EXPECT_EQ(OkStatus(), writer.Open()); + EXPECT_EQ(OkStatus(), writer.Write(write_data)); + + // Use a new blob and writer for the resume to simulate having + // crashed/rebooted and starting fresh. + + kvs::ChecksumCrc16 second_checksum; + BlobStoreBuffer second_blob("TestBlobBlock", + partition_, + &second_checksum, + kvs::TestKvs(), + kBufferSize); + EXPECT_EQ(OkStatus(), second_blob.Init()); + BlobStore::BlobWriterWithBuffer second_writer(second_blob); + + StatusWithSize resume_sws = second_writer.Resume(); + EXPECT_EQ(OkStatus(), resume_sws.status()); + const size_t expected_resume_size = (kSectorCount - 2) * kSectorSize; + EXPECT_EQ(expected_resume_size, resume_sws.size()); + StatusWithSize eod_sws = partition_.EndOfWrittenData(); + EXPECT_EQ(OkStatus(), eod_sws.status()); + EXPECT_EQ(eod_sws.size(), resume_sws.size()); + VerifyFlash(flash_.buffer().first(resume_sws.size())); + + EXPECT_EQ( + OkStatus(), + second_writer.Write(span(source_buffer_).subspan(resume_sws.size()))); + EXPECT_EQ(kBlobDataSize, second_writer.CurrentSizeBytes()); + EXPECT_EQ(OkStatus(), second_writer.Close()); + + VerifyBlob(second_blob, kBlobDataSize); +} + +TEST_F(BlobStoreTest, ResumeAbandonBlobBackedUpToZero) { + InitSourceBufferToRandom(0x11309); + + kvs::ChecksumCrc16 checksum; + constexpr size_t kBufferSize = 256; + BlobStoreBuffer blob( + "TestBlobBlock", partition_, &checksum, kvs::TestKvs(), kBufferSize); + EXPECT_EQ(OkStatus(), blob.Init()); + + // The test fills 1/2 sector size less than the full blob size. Resume will + // backup 1.5 sectors for the resume point. Make sure the blob has enough + // sectors to do all that. + ASSERT_GE(kSectorCount, 3U); + + const size_t write_size_bytes = kSectorSize + (kSectorSize / 2); + ConstByteSpan write_data = span(source_buffer_).first(write_size_bytes); + + // Write 1.5 sectors, should resume with zero bytes. + BlobStore::BlobWriterWithBuffer writer(blob); + EXPECT_EQ(OkStatus(), writer.Open()); + EXPECT_EQ(OkStatus(), writer.Write(write_data)); + EXPECT_EQ(OkStatus(), writer.Abandon()); + StatusWithSize resume_sws = writer.Resume(); + EXPECT_EQ(OkStatus(), resume_sws.status()); + EXPECT_EQ(0U, resume_sws.size()); + + // Write 0.5 sectors, should resume with zero bytes. + EXPECT_EQ(OkStatus(), writer.Erase()); + EXPECT_EQ(OkStatus(), writer.Write(write_data.first(kSectorSize / 2))); + EXPECT_EQ(OkStatus(), writer.Abandon()); + resume_sws = writer.Resume(); + EXPECT_EQ(OkStatus(), resume_sws.status()); + EXPECT_EQ(0U, resume_sws.size()); +} } // namespace } // namespace pw::blob_store diff --git a/pw_blob_store/docs.rst b/pw_blob_store/docs.rst index e8810145b3..0428d99a35 100644 --- a/pw_blob_store/docs.rst +++ b/pw_blob_store/docs.rst @@ -42,8 +42,9 @@ Writing to a BlobStore ``BlobWriter`` objects are ``pw::stream::Writer`` compatible, but do not support reading any of the blob's contents. Opening a ``BlobWriter`` on a ``BlobStore`` that contains data will discard any existing data if ``Discard()``, ``Write -()``, or ``Erase()`` are called. There is currently no mechanism to allow -appending to existing data. +()``, or ``Erase()`` are called. Partially written blobs can be resumed using +``Resume()``. There is currently no mechanism to allow appending to completed +data write. .. code-block:: cpp @@ -57,6 +58,28 @@ appending to existing data. // BlobWriter enables error handling on Close() failure. writer.Close(); +Resuming a BlobStore write +-------------------------- +``BlobWriter::Resume()`` supports resuming writes for blobs that have not been +completed (``writer.Close()``). Supported resume situations are after ``Abandon()`` +has been called on a write or after a crash/reboot with a fresh ``BlobStore`` +instance. + +``Resume()`` opens the ``BlobWriter`` at the most recent safe resume point. +``Resume()`` finds the furthest written point in the flash partition and then backs +up by erasing any partially written sector plus a full sector. This backing up is to +try to avoid any corrupted or otherwise wrong data that might have resulted from the +previous write failing. ``Resume()`` returns the current number of valid bytes in the +resumed write. + +If the blob is using a ChecksumAlgorithm, the checksum of the resumed blob write instance +calculated from the content of the already written data. If it is desired to check the +integrity of the already written data, ``BlobWriter::CurrentChecksum()`` can be used to +check against the incoming data. + +Once ``Resume()`` has successfully completed, the writer is ready to continue writing +as normal. + Erasing a BlobStore =================== There are two distinctly different mechanisms to "erase" the contents of a BlobStore: diff --git a/pw_blob_store/public/pw_blob_store/blob_store.h b/pw_blob_store/public/pw_blob_store/blob_store.h index d87621e6c2..f4aa990145 100644 --- a/pw_blob_store/public/pw_blob_store/blob_store.h +++ b/pw_blob_store/public/pw_blob_store/blob_store.h @@ -77,10 +77,10 @@ class BlobStore { return max_file_name_size + sizeof(internal::BlobMetadataHeader); } - // Open a blob for writing/erasing. Open will invalidate any existing blob - // that may be stored, and will not retain the previous file name. Can not - // open when already open. Only one writer is allowed to be open at a time. - // Returns: + // Open writer for writing/erasing of a new/fresh blob. Open will invalidate + // any existing blob that may be stored, and will not retain the previous + // file name. Can not open when any readers or writers are already open for + // this blob. Only one writer is allowed to be open at a time. Returns: // // Preconditions: // This writer must not already be open. @@ -92,30 +92,86 @@ class BlobStore { // already open. Status Open(); - // Finalize a blob write. Flush all remaining buffered data to storage and - // store blob metadata. Close fails in the closed state, do NOT retry Close - // on error. An error may or may not result in an invalid blob stored. + // Open and resume an in-progress/interrupted blob for writing. This will + // check for any existing write-in-prgress blob (not Closed) that may be + // partially stored, interrupted by a crash, reboot, or other reason. + // Resume writing at the last known good write location. + // + // If no in-progress/interrupted blob write is found, a normal Open() for + // writing is performed. + // + // File name in writer instance is retained from the previous Abandon() + // write, but not persisted across new writer instances. + // + // Checksum of resumed write is calculated from data written to flash. + // + // Can not resume when already open. Only one writer is allowed to be open + // at a time. + // + // Current implementation can only resume blob writes that was Abandon() or + // interrupted by crash/reboot. Completed valid blobs are not able to be + // resumed to append additional data (support not implemented at this time). + // + // Preconditions: + // - This writer must not already be open. + // - Blob must not be valid. Opening a completed blob to append is not + // currently supported. + // - This writer's metadata encode buffer must be at + // least the size of internal::BlobMetadataHeader. + // // Returns: + // OK, size - Number of bytes already written in the resumed blob write. + // UNAVAILABLE - Unable to resume, another writer or reader instance is + // already open. + StatusWithSize Resume(); + + // Finalize a completed blob write and change the writer state to closed. + // Flush all remaining buffered data to storage and store blob metadata. + // Close fails in the closed state, do NOT retry Close on error. An error + // may or may not result in an invalid blob stored (depending on the failure + // mode). Returns: // // OK - success. + // FAILED_PRECONDITION - can not close if not open. // DATA_LOSS - Error writing data or fail to verify written data. Status Close(); + // Abandon the current in-progress blob write and change the writer state to + // closed. Blob is left in an invalid data state (no valid data to read). + // + // After Abandon, writer can be opened either with Open to start a new blob + // write or Resume to resume the abandoned blob write. On resume, any bytes + // written to flash at the Abandon point are handled by the resume + // algorithm. + // + // Abandon fails in the closed state, do NOT retry Abandon on error. + // + // OK - successfully + // FAILED_PRECONDITION - not open. + Status Abandon(); + bool IsOpen() { return open_; } - // Erase the blob partition and reset state for a new blob. Explicit calls - // to Erase are optional, beginning a write will do any needed Erase. - // Returns: + // Erase the blob partition and reset state for a new blob, keeping the + // writer in the opened state, Explicit calls to Erase are optional, + // beginning a write will do any needed Erase. The Erase process includes + // doing a Discard of any in-progress blob write but keeps the writer in the + // opened state. Returns: // // OK - success. - // UNAVAILABLE - Unable to erase while reader is open. + // FAILED_PRECONDITION - not open. // [error status] - flash erase failed. Status Erase() { return open_ ? store_.Erase() : Status::FailedPrecondition(); } - // Discard the current blob. Any written bytes to this point are considered - // invalid. Returns: + // Discard the current blob write and keep the writer in the opened state, + // ready to start a new/clean blob write. Any written bytes to this point + // are considered invalid and discarded. + // + // Discard does not directly perform an erase. Erase of the already written + // data happens either by an explicit Erase command or on-demand when first + // new data is written after the Discard. Returns: // // OK - success. // FAILED_PRECONDITION - not open. @@ -140,10 +196,34 @@ class BlobStore { // buffer, file name not set. Status SetFileName(std::string_view file_name); + // Copies the file name associated with the data written to `dest`, and + // returns the number of bytes written to the destination buffer. The string + // is not null-terminated. + // + // Returns: + // OK - File name copied, size contains file name length. + // RESOURCE_EXHAUSTED - `dest` too small to fit file name, size contains + // first N bytes of the file name. + // NOT_FOUND - No file name set for this blob. + // FAILED_PRECONDITION - not open + // + StatusWithSize GetFileName(span dest); + size_t CurrentSizeBytes() const { return open_ ? store_.write_address_ : 0; } + // Get the current running checksum for the current write session. + // + // Returns: + // ConstByteSpan - current checksum + // NOT_FOUND - No checksum algo in use + // FAILED_PRECONDITION - not open + // + Result CurrentChecksum() const { + return Status::Unimplemented(); + } + // Max file name length, not including null terminator (null terminators // are not stored). size_t MaxFileNameLength() { @@ -400,6 +480,13 @@ class BlobStore { // already open. Status OpenWrite(); + // Open to resume a blob write. Returns: + // + // size - Number of bytes already written in the resumed blob write. + // UNAVAILABLE - Unable to open writer, another writer or reader instance is + // already open. + StatusWithSize ResumeWrite(); + // Open to do a blob read. Returns: // // OK - success. @@ -528,7 +615,7 @@ class BlobStore { Status ValidateChecksum(size_t blob_size_bytes, internal::ChecksumValue expected); - Status CalculateChecksumFromFlash(size_t bytes_to_check); + Status CalculateChecksumFromFlash(size_t bytes_to_check, bool finish = true); const std::string_view MetadataKey() const { return name_; }