commit 91627487e49c32e6be0102a4531aba0ed4255f0c
author David Rogers <davidrogers@google.com> Thu Feb 27 17:38:12 2020 -0800
committer CQ Bot Account <commit-bot@chromium.org> Fri Feb 28 20:24:05 2020 +0000
tree 82b084737f8abdde54dab8fe130d38d1304b90e7
parent cd87c32d672d72c25a594f42ab8e60f4ea50f2ca

pw_kvs: On init ensure there is at least one empty sector

As part of the init process make sure there is at least one empty
sector. If there is not, attempt to garbage collect one. If that fails,
fail init.

Change-Id: Ie8cad38fe14a36eb199feaba99b9772b776542ce

pw_kvs/key_value_store.cc

diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 415544f..45bf2e4 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -79,6 +79,7 @@
 
   size_t total_corrupt_bytes = 0;
   int corrupt_entries = 0;
+  bool empty_sector_found = false;
 
   for (SectorDescriptor& sector : sectors_) {
     Address entry_address = sector_address;
@@ -154,6 +155,9 @@
           sector_corrupt_bytes);
     }
 
+    if (sector.Empty(sector_size_bytes)) {
+      empty_sector_found = true;
+    }
     sector_address += sector_size_bytes;
     total_corrupt_bytes += sector_corrupt_bytes;
   }
@@ -179,6 +183,16 @@
     last_new_sector_ = SectorFromKey(newest_key);
   }
 
+  if (!empty_sector_found) {
+    // TODO: Record/report the error condition and recovery result.
+    Status gc_result = GarbageCollectPartial();
+
+    if (!gc_result.ok()) {
+      ERR("KVS init failed: Unable to maintain required free sector");
+      return Status::INTERNAL;
+    }
+  }
+
   initialized_ = true;
 
   INF("KeyValueStore init complete: active keys %zu, deleted keys %zu, sectors "

pw_kvs/key_value_store_error_handling_test.cc

diff --git a/pw_kvs/key_value_store_error_handling_test.cc b/pw_kvs/key_value_store_error_handling_test.cc
index 0fb59c4..a13010d 100644
--- a/pw_kvs/key_value_store_error_handling_test.cc
+++ b/pw_kvs/key_value_store_error_handling_test.cc
@@ -217,6 +217,25 @@
   EXPECT_EQ(0u, stats.writable_bytes);
 }
 
+TEST_F(KvsErrorHandling, Init_CorruptSectors_ShouldRecoverOne) {
+  InitFlashTo(AsBytes(kEntry1, kEntry2));
+
+  // Corrupt all of the 4 512-byte flash sectors. Leave the pre-init entries
+  // intact. A corrupt sector without entries should be GC'ed on init because
+  // the KVS must maintain one empty sector at all times.
+  flash_.buffer()[64] = byte(0xef);
+  flash_.buffer()[513] = byte(0xef);
+  flash_.buffer()[1025] = byte(0xef);
+  flash_.buffer()[1537] = byte(0xef);
+
+  ASSERT_EQ(Status::DATA_LOSS, kvs_.Init());
+
+  auto stats = kvs_.GetStorageStats();
+  EXPECT_EQ(64u, stats.in_use_bytes);
+  EXPECT_EQ(3 * 512u - 64u, stats.reclaimable_bytes);
+  EXPECT_EQ(0u, stats.writable_bytes);
+}
+
 TEST_F(KvsErrorHandling, Init_CorruptKey_RevertsToPreviousVersion) {
   constexpr auto kVersion7 =
       MakeValidEntry(kMagic, 7, "my_key", ByteStr("version 7"));