pw_blob_store: Add support for deferred writes
- Add support for deferred writes.
- Use DeferredWriter that is similar to BlobWriter, but does deferred
writes.
- Decouple write buffer size from flash write size.
- Return DATA_LOSS for all write errors and treat blob as unwritable
until erased.
Change-Id: I4eeb763875560d058188687e19d61f8ee4582677
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/17641
Commit-Queue: David Rogers <davidrogers@google.com>
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Reviewed-by: Keir Mierle <keir@google.com>
diff --git a/pw_blob_store/BUILD b/pw_blob_store/BUILD
index dc8689f..188ede7 100644
--- a/pw_blob_store/BUILD
+++ b/pw_blob_store/BUILD
@@ -53,3 +53,19 @@
"//pw_unit_test",
],
)
+
+pw_cc_test(
+ name = "blob_store_deferred_write_test",
+ srcs = [
+ "blob_store_deferred_write_test.cc",
+ ],
+ deps = [
+ ":pw_blob_store",
+ "//pw_kvs:crc16",
+ "//pw_kvs:fake_flash",
+ "//pw_kvs:fake_flash_test_key_value_store",
+ "//pw_log",
+ "//pw_random",
+ "//pw_unit_test",
+ ],
+)
diff --git a/pw_blob_store/BUILD.gn b/pw_blob_store/BUILD.gn
index 4576ff4..99c2e03 100644
--- a/pw_blob_store/BUILD.gn
+++ b/pw_blob_store/BUILD.gn
@@ -39,7 +39,10 @@
}
pw_test_group("tests") {
- tests = [ ":blob_store_test" ]
+ tests = [
+ ":blob_store_test",
+ ":blob_store_deferred_write_test",
+ ]
}
pw_test("blob_store_test") {
@@ -54,6 +57,18 @@
sources = [ "blob_store_test.cc" ]
}
+pw_test("blob_store_deferred_write_test") {
+ deps = [
+ ":pw_blob_store",
+ "$dir_pw_kvs:crc16",
+ "$dir_pw_kvs:fake_flash",
+ "$dir_pw_kvs:fake_flash_test_key_value_store",
+ dir_pw_log,
+ dir_pw_random,
+ ]
+ sources = [ "blob_store_deferred_write_test.cc" ]
+}
+
pw_doc_group("docs") {
sources = [ "docs.rst" ]
}
diff --git a/pw_blob_store/blob_store.cc b/pw_blob_store/blob_store.cc
index 27da48a..d594d3f 100644
--- a/pw_blob_store/blob_store.cc
+++ b/pw_blob_store/blob_store.cc
@@ -17,6 +17,7 @@
#include <algorithm>
#include "pw_log/log.h"
+#include "pw_status/try.h"
namespace pw::blob_store {
@@ -30,12 +31,13 @@
PW_CHECK_UINT_EQ((write_buffer_size_alignment), 0);
PW_CHECK_UINT_GE(write_buffer_.size_bytes(), partition_.alignment_bytes());
PW_CHECK_UINT_LE(write_buffer_.size_bytes(), partition_.sector_size_bytes());
+ PW_CHECK_UINT_GE(write_buffer_.size_bytes(), flash_write_size_bytes_);
+ PW_CHECK_UINT_GE(flash_write_size_bytes_, partition_.alignment_bytes());
bool erased = false;
if (partition_.IsErased(&erased).ok() && erased) {
flash_erased_ = true;
- } else {
- flash_erased_ = false;
+ valid_data_ = true;
}
ResetChecksum();
@@ -91,48 +93,59 @@
}
Status BlobStore::CloseWrite() {
- PW_DCHECK(writer_open_);
- Status status = Status::OK;
-
- // Only need to finish a blob if the blob has any bytes.
- if (write_address_ > 0) {
- // Flush bytes in buffer.
- if (write_address_ != flash_address_) {
- PW_CHECK_UINT_GE(write_address_, flash_address_);
- size_t bytes_in_buffer = write_address_ - flash_address_;
-
- // Zero out the remainder of the buffer.
- auto zero_span = write_buffer_.subspan(bytes_in_buffer);
- std::memset(zero_span.data(), 0, zero_span.size_bytes());
-
- status = CommitToFlash(write_buffer_, bytes_in_buffer);
+ auto do_close_write = [&]() -> Status {
+ // If not valid to write, there was data loss and the close will result in a
+ // not valid blob. Don't need to flush any write buffered bytes.
+ if (!ValidToWrite()) {
+ return Status::DATA_LOSS;
}
- if (status.ok()) {
- // Buffer should be clear unless there was a write error.
- PW_DCHECK_UINT_EQ(flash_address_, write_address_);
- // Should not be completing a blob write when it has completed metadata.
- PW_DCHECK(!metadata_.complete);
-
- metadata_ = {
- .checksum = 0, .data_size_bytes = flash_address_, .complete = true};
-
- if (checksum_algo_ != nullptr) {
- ConstByteSpan checksum = checksum_algo_->Finish();
- std::memcpy(&metadata_.checksum,
- checksum.data(),
- std::min(checksum.size(), sizeof(metadata_.checksum)));
- }
-
- status = kvs_.Put(MetadataKey(), metadata_);
+ if (write_address_ == 0) {
+ return Status::OK;
}
- }
+
+ PW_LOG_DEBUG("Close with %u bytes in write buffer",
+ static_cast<unsigned>(WriteBufferBytesUsed()));
+
+ // Do a Flush of any flash_write_size_bytes_ sized chunks so any remaining
+ // bytes in the write buffer are less than flash_write_size_bytes_.
+ PW_TRY(Flush());
+
+ // If any bytes remain in buffer it is because it is a chunk less than
+ // flash_write_size_bytes_. Pad the chunk to flash_write_size_bytes_ and
+ // write it to flash.
+ if (!WriteBufferEmpty()) {
+ PW_TRY(FlushFinalPartialChunk());
+ }
+
+ // If things are still good, save the blob metadata.
+ // Should not be completing a blob write when it has completed metadata.
+ PW_DCHECK(WriteBufferEmpty());
+ PW_DCHECK(!metadata_.complete);
+
+ metadata_ = {
+ .checksum = 0, .data_size_bytes = flash_address_, .complete = true};
+ if (checksum_algo_ != nullptr) {
+ ConstByteSpan checksum = checksum_algo_->Finish();
+ std::memcpy(&metadata_.checksum,
+ checksum.data(),
+ std::min(checksum.size(), sizeof(metadata_.checksum)));
+ }
+
+ PW_TRY(kvs_.Put(MetadataKey(), metadata_));
+ PW_TRY(ValidateChecksum());
+
+ return Status::OK;
+ };
+
+ const Status status = do_close_write();
writer_open_ = false;
- if (status.ok()) {
- return ValidateChecksum();
+ if (!status.ok()) {
+ valid_data_ = false;
+ return Status::DATA_LOSS;
}
- return Status::DATA_LOSS;
+ return Status::OK;
}
Status BlobStore::CloseRead() {
@@ -142,8 +155,9 @@
}
Status BlobStore::Write(ConstByteSpan data) {
- PW_DCHECK(writer_open_);
-
+ if (!ValidToWrite()) {
+ return Status::DATA_LOSS;
+ }
if (data.size_bytes() == 0) {
return Status::OK;
}
@@ -154,8 +168,10 @@
return Status::RESOURCE_EXHAUSTED;
}
- if (write_address_ == 0 && !erased()) {
- Erase();
+ Status status = EraseIfNeeded();
+ // TODO: switch to TRY once available.
+ if (!status.ok()) {
+ return Status::DATA_LOSS;
}
// Write in (up to) 3 steps:
@@ -166,22 +182,26 @@
// Step 1) If there is any data in the write buffer, finish filling write
// buffer and if full write it to flash.
- const size_t write_chunk_bytes = write_buffer_.size_bytes();
if (!WriteBufferEmpty()) {
- PW_CHECK_UINT_GE(write_address_, flash_address_);
- size_t bytes_in_buffer = write_address_ - flash_address_;
- PW_CHECK_UINT_GE(write_chunk_bytes, bytes_in_buffer);
+ size_t bytes_in_buffer = WriteBufferBytesUsed();
- size_t buffer_remaining = write_chunk_bytes - bytes_in_buffer;
+ // Non-deferred writes only use the first flash_write_size_bytes_ of the
+ // write buffer to buffer writes less than flash_write_size_bytes_.
+ PW_CHECK_UINT_GT(flash_write_size_bytes_, bytes_in_buffer);
- // Add bytes up to filling the write buffer.
+ // Not using WriteBufferBytesFree() because non-deferred writes (which
+ // is this method) only use the first flash_write_size_bytes_ of the write
+ // buffer.
+ size_t buffer_remaining = flash_write_size_bytes_ - bytes_in_buffer;
+
+ // Add bytes up to filling the flash write size.
size_t add_bytes = std::min(buffer_remaining, data.size_bytes());
std::memcpy(write_buffer_.data() + bytes_in_buffer, data.data(), add_bytes);
write_address_ += add_bytes;
bytes_in_buffer += add_bytes;
data = data.subspan(add_bytes);
- if (bytes_in_buffer != write_chunk_bytes) {
+ if (bytes_in_buffer != flash_write_size_bytes_) {
// If there was not enough bytes to finish filling the write buffer, there
// should not be any bytes left.
PW_DCHECK(data.size_bytes() == 0);
@@ -190,8 +210,9 @@
// The write buffer is full, flush to flash.
Status status = CommitToFlash(write_buffer_);
+ // TODO: switch to TRY once available.
if (!status.ok()) {
- return status;
+ return Status::DATA_LOSS;
}
PW_DCHECK(WriteBufferEmpty());
@@ -202,17 +223,17 @@
// Step 2) Write as many block-sized chunks as the data has remaining after
// step 1.
- // TODO: Decouple write buffer size from optimal bulk write size.
- while (data.size_bytes() >= write_chunk_bytes) {
+ while (data.size_bytes() >= flash_write_size_bytes_) {
PW_DCHECK(WriteBufferEmpty());
- write_address_ += write_chunk_bytes;
- Status status = CommitToFlash(data.first(write_chunk_bytes));
+ write_address_ += flash_write_size_bytes_;
+ Status status = CommitToFlash(data.first(flash_write_size_bytes_));
+ // TODO: switch to TRY once available.
if (!status.ok()) {
- return status;
+ return Status::DATA_LOSS;
}
- data = data.subspan(write_chunk_bytes);
+ data = data.subspan(flash_write_size_bytes_);
}
// step 3) Put any remaining bytes to the buffer. Put the bytes starting at
@@ -228,6 +249,130 @@
return Status::OK;
}
+Status BlobStore::AddToWriteBuffer(ConstByteSpan data) {
+ if (!ValidToWrite()) {
+ return Status::DATA_LOSS;
+ }
+ if (WriteBytesRemaining() == 0) {
+ return Status::OUT_OF_RANGE;
+ }
+ if (WriteBufferBytesFree() < data.size_bytes()) {
+ return Status::RESOURCE_EXHAUSTED;
+ }
+
+ size_t bytes_in_buffer = WriteBufferBytesUsed();
+
+ std::memcpy(
+ write_buffer_.data() + bytes_in_buffer, data.data(), data.size_bytes());
+ write_address_ += data.size_bytes();
+
+ return Status::OK;
+}
+
+Status BlobStore::Flush() {
+ if (!ValidToWrite()) {
+ return Status::DATA_LOSS;
+ }
+ if (WriteBufferBytesUsed() == 0) {
+ return Status::OK;
+ }
+ // Don't need to check available space, AddToWriteBuffer() will not enqueue
+ // more than can be written to flash.
+
+ Status status = EraseIfNeeded();
+ // TODO: switch to TRY once available.
+ if (!status.ok()) {
+ return Status::DATA_LOSS;
+ }
+
+ ByteSpan data = std::span(write_buffer_.data(), WriteBufferBytesUsed());
+ while (data.size_bytes() >= flash_write_size_bytes_) {
+ Status status = CommitToFlash(data.first(flash_write_size_bytes_));
+ // TODO: switch to TRY once available.
+ if (!status.ok()) {
+ return Status::DATA_LOSS;
+ }
+
+ data = data.subspan(flash_write_size_bytes_);
+ }
+
+ // Only a multiple of flash_write_size_bytes_ are written in the flush. Any
+ // remainder is held until later for either a flush with
+ // flash_write_size_bytes buffered or the writer is closed.
+ if (!WriteBufferEmpty()) {
+ PW_DCHECK_UINT_EQ(data.size_bytes(), WriteBufferBytesUsed());
+ // For any leftover bytes less than the flash write size, move them to the
+ // start of the bufer.
+ std::memmove(write_buffer_.data(), data.data(), data.size_bytes());
+ } else {
+ PW_DCHECK_UINT_EQ(data.size_bytes(), 0);
+ }
+
+ return Status::OK;
+}
+
+Status BlobStore::FlushFinalPartialChunk() {
+ size_t bytes_in_buffer = WriteBufferBytesUsed();
+
+ PW_DCHECK_UINT_GT(bytes_in_buffer, 0);
+ PW_DCHECK_UINT_LE(bytes_in_buffer, flash_write_size_bytes_);
+ PW_DCHECK_UINT_LE(flash_write_size_bytes_, WriteBytesRemaining());
+
+ PW_LOG_DEBUG(
+ " Remainder %u bytes in write buffer to zero-pad to flash write "
+ "size and commit",
+ static_cast<unsigned>(bytes_in_buffer));
+
+ // Zero out the remainder of the buffer.
+ auto zero_span = write_buffer_.subspan(bytes_in_buffer);
+ std::memset(zero_span.data(), 0, zero_span.size_bytes());
+ // TODO: look in to using flash erased value for fill, to possibly allow
+ // better resuming of writing.
+
+ ConstByteSpan remaining_bytes = write_buffer_.first(flash_write_size_bytes_);
+ return CommitToFlash(remaining_bytes, bytes_in_buffer);
+}
+
+Status BlobStore::CommitToFlash(ConstByteSpan source, size_t data_bytes) {
+ if (data_bytes == 0) {
+ data_bytes = source.size_bytes();
+ }
+ flash_erased_ = false;
+ valid_data_ = true;
+ StatusWithSize result = partition_.Write(flash_address_, source);
+ flash_address_ += data_bytes;
+ if (checksum_algo_ != nullptr) {
+ checksum_algo_->Update(source.first(data_bytes));
+ }
+
+ if (!result.status().ok()) {
+ valid_data_ = false;
+ }
+
+ return result.status();
+}
+
+// Needs to be in .cc file since PW_CHECK doesn't like being in .h files.
+size_t BlobStore::WriteBufferBytesUsed() const {
+ PW_CHECK_UINT_GE(write_address_, flash_address_);
+ return write_address_ - flash_address_;
+}
+
+// Needs to be in .cc file since PW_DCHECK doesn't like being in .h files.
+size_t BlobStore::WriteBufferBytesFree() const {
+ PW_DCHECK_UINT_GE(write_buffer_.size_bytes(), WriteBufferBytesUsed());
+ size_t buffer_remaining = write_buffer_.size_bytes() - WriteBufferBytesUsed();
+ return std::min(buffer_remaining, WriteBytesRemaining());
+}
+
+Status BlobStore::EraseIfNeeded() {
+ if (flash_address_ == 0) {
+ // Always just erase. Erase is smart enough to only erase if needed.
+ return Erase();
+ }
+ return Status::OK;
+}
+
StatusWithSize BlobStore::Read(size_t offset, ByteSpan dest) {
(void)offset;
(void)dest;
@@ -235,7 +380,7 @@
}
Result<ByteSpan> BlobStore::GetMemoryMappedBlob() const {
- if (write_address_ == 0 || !valid_data_) {
+ if (flash_address_ == 0 || !valid_data_) {
return Status::FAILED_PRECONDITION;
}
@@ -249,8 +394,6 @@
size_t BlobStore::ReadableDataBytes() const { return flash_address_; }
Status BlobStore::Erase() {
- PW_DCHECK(writer_open_);
-
// Can't erase if any readers are open, since this would erase flash right out
// from under them.
if (readers_open_ != 0) {
@@ -259,9 +402,10 @@
// If already erased our work here is done.
if (flash_erased_) {
- PW_DCHECK_UINT_EQ(write_address_, 0);
+ // The write buffer might have bytes when the erase happens.
+ PW_DCHECK_UINT_LE(write_address_, write_buffer_.size_bytes());
PW_DCHECK_UINT_EQ(flash_address_, 0);
- PW_DCHECK(!valid_data_);
+ PW_DCHECK(valid_data_);
return Status::OK;
}
@@ -271,6 +415,7 @@
if (status.ok()) {
flash_erased_ = true;
+ valid_data_ = true;
}
return status;
}
@@ -303,8 +448,8 @@
}
PW_LOG_DEBUG("Validate checksum of 0x%08x in flash for blob of %u bytes",
- unsigned(metadata_.checksum),
- unsigned(metadata_.data_size_bytes));
+ static_cast<unsigned>(metadata_.checksum),
+ static_cast<unsigned>(metadata_.data_size_bytes));
auto result = CalculateChecksumFromFlash(metadata_.data_size_bytes);
if (!result.ok()) {
return result.status();
@@ -330,6 +475,7 @@
const size_t read_size = std::min(size_t(end - address), buffer.size());
StatusWithSize status =
partition_.Read(address, std::span(buffer).first(read_size));
+ // TODO: switch to TRY once available.
if (!status.ok()) {
return status.status();
}
diff --git a/pw_blob_store/blob_store_deferred_write_test.cc b/pw_blob_store/blob_store_deferred_write_test.cc
new file mode 100644
index 0000000..8100f2a
--- /dev/null
+++ b/pw_blob_store/blob_store_deferred_write_test.cc
@@ -0,0 +1,178 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+#include <array>
+#include <cstddef>
+#include <cstring>
+#include <span>
+
+#include "gtest/gtest.h"
+#include "pw_blob_store/blob_store.h"
+#include "pw_kvs/crc16_checksum.h"
+#include "pw_kvs/fake_flash_memory.h"
+#include "pw_kvs/flash_memory.h"
+#include "pw_kvs/test_key_value_store.h"
+#include "pw_log/log.h"
+#include "pw_random/xor_shift.h"
+
+namespace pw::blob_store {
+namespace {
+
+class DeferredWriteTest : public ::testing::Test {
+ protected:
+ DeferredWriteTest() : flash_(kFlashAlignment), partition_(&flash_) {}
+
+ void InitFlashTo(std::span<const std::byte> contents) {
+ partition_.Erase();
+ std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
+ }
+
+ void InitBufferToRandom(uint64_t seed) {
+ partition_.Erase();
+ random::XorShiftStarRng64 rng(seed);
+ rng.Get(buffer_);
+ }
+
+ void InitBufferToFill(char fill) {
+ partition_.Erase();
+ std::memset(buffer_.data(), fill, buffer_.size());
+ }
+
+ // Fill the source buffer with random pattern based on given seed, written to
+ // BlobStore in specified chunk size.
+ void ChunkWriteTest(size_t chunk_size, size_t flush_interval) {
+ constexpr size_t kBufferSize = 256;
+ constexpr size_t kWriteSize = 64;
+ kvs::ChecksumCrc16 checksum;
+
+ size_t bytes_since_flush = 0;
+
+ char name[16] = {};
+ snprintf(name, sizeof(name), "Blob%u", static_cast<unsigned>(chunk_size));
+
+ BlobStoreBuffer<kBufferSize> blob(
+ name, &partition_, &checksum, kvs::TestKvs(), kWriteSize);
+ EXPECT_EQ(Status::OK, blob.Init());
+
+ BlobStore::DeferredWriter writer(blob);
+ EXPECT_EQ(Status::OK, writer.Open());
+
+ ByteSpan source = buffer_;
+ while (source.size_bytes() > 0) {
+ const size_t write_size = std::min(source.size_bytes(), chunk_size);
+
+ PW_LOG_DEBUG("Do write of %u bytes, %u bytes remain",
+ static_cast<unsigned>(write_size),
+ static_cast<unsigned>(source.size_bytes()));
+
+ EXPECT_EQ(Status::OK, writer.Write(source.first(write_size)));
+ // TODO: Add check that the write did not go to flash yet.
+
+ source = source.subspan(write_size);
+ bytes_since_flush += write_size;
+
+ if (bytes_since_flush >= flush_interval) {
+ bytes_since_flush = 0;
+ EXPECT_EQ(Status::OK, writer.Flush());
+ }
+ }
+
+ EXPECT_EQ(Status::OK, writer.Close());
+
+ // Use reader to check for valid data.
+ BlobStore::BlobReader reader(blob, 0);
+ EXPECT_EQ(Status::OK, reader.Open());
+ Result<ByteSpan> result = reader.GetMemoryMappedBlob();
+ ASSERT_TRUE(result.ok());
+ VerifyFlash(result.value());
+ EXPECT_EQ(Status::OK, reader.Close());
+ }
+
+ void VerifyFlash(ByteSpan verify_bytes) {
+ // Should be defined as same size.
+ EXPECT_EQ(buffer_.size(), flash_.buffer().size_bytes());
+
+ // Can't allow it to march off the end of buffer_.
+ ASSERT_LE(verify_bytes.size_bytes(), buffer_.size());
+
+ for (size_t i = 0; i < verify_bytes.size_bytes(); i++) {
+ EXPECT_EQ(buffer_[i], verify_bytes[i]);
+ }
+ }
+
+ static constexpr size_t kFlashAlignment = 16;
+ static constexpr size_t kSectorSize = 2048;
+ static constexpr size_t kSectorCount = 2;
+
+ kvs::FakeFlashMemoryBuffer<kSectorSize, kSectorCount> flash_;
+ kvs::FlashPartition partition_;
+ std::array<std::byte, kSectorCount * kSectorSize> buffer_;
+};
+
+TEST_F(DeferredWriteTest, ChunkWrite1) {
+ InitBufferToRandom(0x8675309);
+ ChunkWriteTest(1, 16);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite2) {
+ InitBufferToRandom(0x8675);
+ ChunkWriteTest(2, 16);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite3) {
+ InitBufferToFill(0);
+ ChunkWriteTest(3, 16);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite4) {
+ InitBufferToFill(1);
+ ChunkWriteTest(4, 64);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite5) {
+ InitBufferToFill(0xff);
+ ChunkWriteTest(5, 64);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite16) {
+ InitBufferToRandom(0x86);
+ ChunkWriteTest(16, 128);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite64) {
+ InitBufferToRandom(0x9);
+ ChunkWriteTest(64, 128);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite64FullBufferFill) {
+ InitBufferToRandom(0x9);
+ ChunkWriteTest(64, 256);
+}
+
+TEST_F(DeferredWriteTest, ChunkWrite256) {
+ InitBufferToRandom(0x12345678);
+ ChunkWriteTest(256, 256);
+}
+
+// TODO: test that has dirty flash, invalidated blob, open writer, invalidate
+// (not erase) and start writing (does the auto/implicit erase).
+
+// TODO: test that has dirty flash, invalidated blob, open writer, explicit
+// erase and start writing.
+
+// TODO: test start with dirty flash/invalid blob, open writer, write, close.
+// Verifies erase logic when write buffer has contents.
+
+} // namespace
+} // namespace pw::blob_store
diff --git a/pw_blob_store/blob_store_test.cc b/pw_blob_store/blob_store_test.cc
index 396a54b..97e8ce4 100644
--- a/pw_blob_store/blob_store_test.cc
+++ b/pw_blob_store/blob_store_test.cc
@@ -39,22 +39,25 @@
std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
}
- void InitBufferToRandom(uint64_t seed) {
+ void InitSourceBufferToRandom(uint64_t seed) {
partition_.Erase();
random::XorShiftStarRng64 rng(seed);
- rng.Get(buffer_);
+ rng.Get(source_buffer_);
+ }
+
+ void InitSourceBufferToFill(char fill) {
+ partition_.Erase();
+ std::memset(source_buffer_.data(), fill, source_buffer_.size());
}
// Fill the source buffer with random pattern based on given seed, written to
// BlobStore in specified chunk size.
- void ChunkWriteTest(size_t chunk_size, uint64_t seed) {
- InitBufferToRandom(seed);
-
+ void ChunkWriteTest(size_t chunk_size) {
constexpr size_t kBufferSize = 256;
kvs::ChecksumCrc16 checksum;
char name[16] = {};
- snprintf(name, sizeof(name), "Blob%u", unsigned(chunk_size));
+ snprintf(name, sizeof(name), "Blob%u", static_cast<unsigned>(chunk_size));
BlobStoreBuffer<kBufferSize> blob(
name, &partition_, &checksum, kvs::TestKvs());
@@ -63,13 +66,13 @@
BlobStore::BlobWriter writer(blob);
EXPECT_EQ(Status::OK, writer.Open());
- ByteSpan source = buffer_;
+ ByteSpan source = source_buffer_;
while (source.size_bytes() > 0) {
const size_t write_size = std::min(source.size_bytes(), chunk_size);
PW_LOG_DEBUG("Do write of %u bytes, %u bytes remain",
- unsigned(write_size),
- unsigned(source.size_bytes()));
+ static_cast<unsigned>(write_size),
+ static_cast<unsigned>(source.size_bytes()));
EXPECT_EQ(Status::OK, writer.Write(source.first(write_size)));
@@ -89,13 +92,13 @@
void VerifyFlash(ByteSpan verify_bytes) {
// Should be defined as same size.
- EXPECT_EQ(buffer_.size(), flash_.buffer().size_bytes());
+ EXPECT_EQ(source_buffer_.size(), flash_.buffer().size_bytes());
- // Can't allow it to march off the end of buffer_.
- ASSERT_LE(verify_bytes.size_bytes(), buffer_.size());
+ // Can't allow it to march off the end of source_buffer_.
+ ASSERT_LE(verify_bytes.size_bytes(), source_buffer_.size());
for (size_t i = 0; i < verify_bytes.size_bytes(); i++) {
- EXPECT_EQ(buffer_[i], verify_bytes[i]);
+ EXPECT_EQ(source_buffer_[i], verify_bytes[i]);
}
}
@@ -105,7 +108,7 @@
kvs::FakeFlashMemoryBuffer<kSectorSize, kSectorCount> flash_;
kvs::FlashPartition partition_;
- std::array<std::byte, kSectorCount * kSectorSize> buffer_;
+ std::array<std::byte, kSectorCount * kSectorSize> source_buffer_;
};
TEST_F(BlobStoreTest, Init_Ok) {
@@ -113,23 +116,86 @@
EXPECT_EQ(Status::OK, blob.Init());
}
-TEST_F(BlobStoreTest, ChunkWrite1) { ChunkWriteTest(1, 0x8675309); }
+TEST_F(BlobStoreTest, MultipleErase) {
+ BlobStoreBuffer<256> blob("Blob_OK", &partition_, nullptr, kvs::TestKvs());
+ EXPECT_EQ(Status::OK, blob.Init());
-TEST_F(BlobStoreTest, ChunkWrite2) { ChunkWriteTest(2, 0x8675); }
+ BlobStore::BlobWriter writer(blob);
+ EXPECT_EQ(Status::OK, writer.Open());
-TEST_F(BlobStoreTest, ChunkWrite16) { ChunkWriteTest(16, 0x86); }
+ EXPECT_EQ(Status::OK, writer.Erase());
+ EXPECT_EQ(Status::OK, writer.Erase());
+ EXPECT_EQ(Status::OK, writer.Erase());
+}
-TEST_F(BlobStoreTest, ChunkWrite64) { ChunkWriteTest(64, 0x9); }
+TEST_F(BlobStoreTest, ChunkWrite1) {
+ InitSourceBufferToRandom(0x8675309);
+ ChunkWriteTest(1);
+}
-TEST_F(BlobStoreTest, ChunkWrite256) { ChunkWriteTest(256, 0x12345678); }
+TEST_F(BlobStoreTest, ChunkWrite2) {
+ InitSourceBufferToRandom(0x8675);
+ ChunkWriteTest(2);
+}
-TEST_F(BlobStoreTest, ChunkWrite512) { ChunkWriteTest(512, 0x42); }
+TEST_F(BlobStoreTest, ChunkWrite3) {
+ InitSourceBufferToFill(0);
+ ChunkWriteTest(3);
+}
-TEST_F(BlobStoreTest, ChunkWrite4096) { ChunkWriteTest(4096, 0x89); }
+TEST_F(BlobStoreTest, ChunkWrite4) {
+ InitSourceBufferToFill(1);
+ ChunkWriteTest(4);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite5) {
+ InitSourceBufferToFill(0xff);
+ ChunkWriteTest(5);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite16) {
+ InitSourceBufferToRandom(0x86);
+ ChunkWriteTest(16);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite64) {
+ InitSourceBufferToRandom(0x9);
+ ChunkWriteTest(64);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite256) {
+ InitSourceBufferToRandom(0x12345678);
+ ChunkWriteTest(256);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite512) {
+ InitSourceBufferToRandom(0x42);
+ ChunkWriteTest(512);
+}
+
+TEST_F(BlobStoreTest, ChunkWrite4096) {
+ InitSourceBufferToRandom(0x89);
+ ChunkWriteTest(4096);
+}
TEST_F(BlobStoreTest, ChunkWriteSingleFull) {
- ChunkWriteTest((kSectorCount * kSectorSize), 0x98765);
+ InitSourceBufferToRandom(0x98765);
+ ChunkWriteTest((kSectorCount * kSectorSize));
}
+// TODO: test that does close with bytes still in the buffer to test zero fill
+// to alignment and write on close.
+
+// TODO: test to do write/close, write/close multiple times.
+
+// TODO: test start with old blob (with KVS entry), open, invalidate, no writes,
+// close. Verify the KVS entry is gone and blob is fully invalid.
+
+// TODO: test that checks doing read after bytes are in write buffer but before
+// any bytes are flushed to flash.
+
+// TODO: test mem mapped read when bytes in write buffer but nothing written to
+// flash.
+
} // namespace
} // namespace pw::blob_store
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 87433f6..14a1461 100644
--- a/pw_blob_store/public/pw_blob_store/blob_store.h
+++ b/pw_blob_store/public/pw_blob_store/blob_store.h
@@ -49,12 +49,15 @@
public:
// Implement the stream::Writer and erase interface for a BlobStore. If not
// already erased, the Write will do any needed erase.
- class BlobWriter final : public stream::Writer {
+ //
+ // Only one writter (of either type) is allowed to be open at a time.
+ // Additionally, writters are unable to open if a reader is already open.
+ class BlobWriter : public stream::Writer {
public:
constexpr BlobWriter(BlobStore& store) : store_(store), open_(false) {}
BlobWriter(const BlobWriter&) = delete;
BlobWriter& operator=(const BlobWriter&) = delete;
- ~BlobWriter() {
+ virtual ~BlobWriter() {
if (open_) {
Close();
}
@@ -95,7 +98,7 @@
// [error status] - flash erase failed.
Status Erase() {
PW_DCHECK(open_);
- return Status::UNIMPLEMENTED;
+ return store_.Erase();
}
// Discard blob (in-progress or valid stored). Any written bytes are
@@ -109,7 +112,8 @@
}
// Probable (not guaranteed) minimum number of bytes at this time that can
- // be written. Returns zero if, in the current state, Write would return
+ // be written. This is not necessarily the full number of bytes remaining in
+ // the blob. Returns zero if, in the current state, Write would return
// status other than OK. See stream.h for additional details.
size_t ConservativeWriteLimit() const override {
PW_DCHECK(open_);
@@ -121,7 +125,7 @@
return store_.write_address_;
}
- private:
+ protected:
Status DoWrite(ConstByteSpan data) override {
PW_DCHECK(open_);
return store_.Write(data);
@@ -131,6 +135,43 @@
bool open_;
};
+ // Implement the stream::Writer and erase interface with deferred action for a
+ // BlobStore. If not already erased, the Flush will do any needed erase.
+ //
+ // Only one writter (of either type) is allowed to be open at a time.
+ // Additionally, writters are unable to open if a reader is already open.
+ class DeferredWriter final : public BlobWriter {
+ public:
+ constexpr DeferredWriter(BlobStore& store) : BlobWriter(store) {}
+ DeferredWriter(const DeferredWriter&) = delete;
+ DeferredWriter& operator=(const DeferredWriter&) = delete;
+ virtual ~DeferredWriter() {}
+
+ // Flush data in the write buffer. Only a multiple of flash_write_size_bytes
+ // are written in the flush. Any remainder is held until later for either
+ // a flush with flash_write_size_bytes buffered or the writer is closed.
+ Status Flush() {
+ PW_DCHECK(open_);
+ return store_.Flush();
+ }
+
+ // Probable (not guaranteed) minimum number of bytes at this time that can
+ // be written. This is not necessarily the full number of bytes remaining in
+ // the blob. Returns zero if, in the current state, Write would return
+ // status other than OK. See stream.h for additional details.
+ size_t ConservativeWriteLimit() const override {
+ PW_DCHECK(open_);
+ // Deferred writes need to fit in the write buffer.
+ return store_.WriteBufferBytesFree();
+ }
+
+ private:
+ Status DoWrite(ConstByteSpan data) override {
+ PW_DCHECK(open_);
+ return store_.AddToWriteBuffer(data);
+ }
+ };
+
// Implement stream::Reader interface for BlobStore.
class BlobReader final : public stream::Reader {
public:
@@ -197,16 +238,31 @@
size_t offset_;
};
+ // BlobStore
+ // name - Name of blob store, used for metadata KVS key
+ // partition - Flash partiton to use for this blob. Blob uses the entire
+ // partition.
+ // checksum_algo - Optional checksum for blob integrity checking. Use nullptr
+ // for no check.
+ // kvs - KVS used for storing blob metadata.
+ // write_buffer - Used for buffering writes. Needs to be at least
+ // flash_write_size_bytes.
+ // flash_write_size_bytes - Size in bytes to use for flash write operations.
+ // This should be chosen to balance optimal write size and required buffer
+ // size. Must be greater than or equal to flash write alignment, less than
+ // or equal to flash sector size.
BlobStore(std::string_view name,
kvs::FlashPartition* partition,
kvs::ChecksumAlgorithm* checksum_algo,
kvs::KeyValueStore& kvs,
- ByteSpan write_buffer)
+ ByteSpan write_buffer,
+ size_t flash_write_size_bytes)
: name_(name),
partition_(*partition),
checksum_algo_(checksum_algo),
kvs_(kvs),
write_buffer_(write_buffer),
+ flash_write_size_bytes_(flash_write_size_bytes),
initialized_(false),
valid_data_(false),
flash_erased_(false),
@@ -231,9 +287,6 @@
private:
typedef uint32_t ChecksumValue;
- // Is the blob erased and ready to write.
- bool erased() const { return flash_erased_; }
-
// Open to do a blob write. Returns:
//
// OK - success.
@@ -251,7 +304,8 @@
// store blob metadata. Returns:
//
// OK - success.
- // FAILED_PRECONDITION - blob is not open.
+ // DATA_LOSS - Error during write (this close or previous write/flush). Blob
+ // is closed and marked as invalid.
Status CloseWrite();
Status CloseRead();
@@ -260,33 +314,70 @@
// not guaranteed to be fully written out to storage on Write return. Returns:
//
// OK - successful write/enqueue of data.
- // FAILED_PRECONDITION - blob is not in an open (in-progress) write state.
// RESOURCE_EXHAUSTED - unable to write all of requested data at this time. No
// data written.
// OUT_OF_RANGE - Writer has been exhausted, similar to EOF. No data written,
// no more will be written.
+ // DATA_LOSS - Error during write (this write or previous write/flush). No
+ // more will be written by following Write calls for current blob (until
+ // erase/new blob started).
Status Write(ConstByteSpan data);
+ // Similar to Write, but instead immediately writing out to flash, it only
+ // buffers the data. A flush or Close is reqired to get bytes writen out to
+ // flash
+ //
+ // OK - successful write/enqueue of data.
+ // RESOURCE_EXHAUSTED - unable to write all of requested data at this time. No
+ // data written.
+ // OUT_OF_RANGE - Writer has been exhausted, similar to EOF. No data written,
+ // no more will be written.
+ // DATA_LOSS - Error during a previous write/flush. No more will be written by
+ // following Write calls for current blob (until erase/new blob started).
+ Status AddToWriteBuffer(ConstByteSpan data);
+
+ // Flush data in the write buffer. Only a multiple of flash_write_size_bytes
+ // are written in the flush. Any remainder is held until later for either a
+ // flush with flash_write_size_bytes buffered or the writer is closed.
+ //
+ // OK - successful write/enqueue of data.
+ // DATA_LOSS - Error during write (this flush or previous write/flush). No
+ // more will be written by following Write calls for current blob (until
+ // erase/new blob started).
+ Status Flush();
+
+ // Flush a chunk of data in the write buffer smaller than
+ // flash_write_size_bytes. This is only for the final flush as part of the
+ // CloseWrite. The partial chunk is padded to flash_write_size_bytes and a
+ // flash_write_size_bytes chunk is written to flash.
+ //
+ // OK - successful write/enqueue of data.
+ // DATA_LOSS - Error during write (this flush or previous write/flush). No
+ // more will be written by following Write calls for current blob (until
+ // erase/new blob started).
+ Status FlushFinalPartialChunk();
+
// Commit data to flash and update flash_address_ with data bytes written. The
// only time data_bytes should be manually specified is for a CloseWrite with
// an unaligned-size chunk remaining in the buffer that has been zero padded
// to alignment.
- Status CommitToFlash(ConstByteSpan source, size_t data_bytes = 0) {
- if (data_bytes == 0) {
- data_bytes = source.size_bytes();
- }
- flash_erased_ = false;
- valid_data_ = true;
- StatusWithSize result = partition_.Write(flash_address_, source);
- flash_address_ += data_bytes;
- if (checksum_algo_ != nullptr) {
- checksum_algo_->Update(source.first(data_bytes));
- }
+ Status CommitToFlash(ConstByteSpan source, size_t data_bytes = 0);
- return result.status();
- }
+ // Blob is valid/OK to write to. Blob is considered valid to write if no data
+ // has been written due to the auto/implicit erase on write start.
+ //
+ // true - Blob is valid and OK to write to.
+ // false - Blob has previously had an error and not valid for writing new
+ // data.
+ bool ValidToWrite() { return (valid_data_ == true) || (write_address_ == 0); }
- bool WriteBufferEmpty() { return flash_address_ == write_address_; }
+ bool WriteBufferEmpty() const { return flash_address_ == write_address_; }
+
+ size_t WriteBufferBytesUsed() const;
+
+ size_t WriteBufferBytesFree() const;
+
+ Status EraseIfNeeded();
// Read valid data. Attempts to read the lesser of output.size_bytes() or
// available bytes worth of data. Returns:
@@ -352,6 +443,11 @@
kvs::KeyValueStore& kvs_;
ByteSpan write_buffer_;
+ // Size in bytes of flash write operations. This should be chosen to balance
+ // optimal write size and required buffer size. Must be GE flash write
+ // alignment, LE flash sector size.
+ const size_t flash_write_size_bytes_;
+
//
// Internal state for Blob store
//
@@ -360,7 +456,8 @@
// Initialization has been done.
bool initialized_;
- // Bytes stored are valid and good.
+ // Bytes stored are valid and good. Blob is OK to read and write to. Set as
+ // soon as is valid, even when bytes written is still 0.
bool valid_data_;
// Blob partition is currently erased and ready to write a new blob.
@@ -391,8 +488,14 @@
explicit BlobStoreBuffer(std::string_view name,
kvs::FlashPartition* partition,
kvs::ChecksumAlgorithm* checksum_algo,
- kvs::KeyValueStore& kvs)
- : BlobStore(name, partition, checksum_algo, kvs, buffer_) {}
+ kvs::KeyValueStore& kvs,
+ size_t flash_write_size_bytes = kBufferSizeBytes)
+ : BlobStore(name,
+ partition,
+ checksum_algo,
+ kvs,
+ buffer_,
+ flash_write_size_bytes) {}
private:
std::array<std::byte, kBufferSizeBytes> buffer_;
