pw_kvs: Count erases + HeavyMaintenance
- Add counting of sectors erased. This count is only since instance
construction.
- Add HeavyMaintenance to be heavy and aggressive about garbage
collecting sectors with valid data.
- Don't erase sectors when doing garbage collection of sectors that are
already erased.
Change-Id: I4fb1647c1920cdc64a7a1029078575068a9156ce
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/20960
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: David Rogers <davidrogers@google.com>
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 71da378..752590d 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -53,7 +53,7 @@
options_(options),
initialized_(InitializationState::kNotInitialized),
error_detected_(false),
- error_stats_({}),
+ internal_stats_({}),
last_transaction_id_(0) {}
Status KeyValueStore::Init() {
@@ -97,12 +97,12 @@
if (options_.recovery != ErrorRecovery::kManual) {
size_t pre_fix_redundancy_errors =
- error_stats_.missing_redundant_entries_recovered;
+ internal_stats_.missing_redundant_entries_recovered;
Status recovery_status = FixErrors();
if (recovery_status.ok()) {
if (metadata_result == Status::OutOfRange()) {
- error_stats_.missing_redundant_entries_recovered =
+ internal_stats_.missing_redundant_entries_recovered =
pre_fix_redundancy_errors;
INF("KVS init: Redundancy level successfully updated");
} else {
@@ -301,9 +301,10 @@
StorageStats stats{};
const size_t sector_size = partition_.sector_size_bytes();
bool found_empty_sector = false;
- stats.corrupt_sectors_recovered = error_stats_.corrupt_sectors_recovered;
+ stats.sector_erase_count = internal_stats_.sector_erase_count;
+ stats.corrupt_sectors_recovered = internal_stats_.corrupt_sectors_recovered;
stats.missing_redundant_entries_recovered =
- error_stats_.missing_redundant_entries_recovered;
+ internal_stats_.missing_redundant_entries_recovered;
for (const SectorDescriptor& sector : sectors_) {
stats.in_use_bytes += sector.valid_bytes();
@@ -867,7 +868,7 @@
return Status::Ok();
}
-Status KeyValueStore::FullMaintenance() {
+Status KeyValueStore::FullMaintenanceHelper(MaintenanceType maintenance_type) {
if (initialized_ == InitializationState::kNotInitialized) {
return Status::FailedPrecondition();
}
@@ -897,7 +898,8 @@
// Is bytes in use over the threshold.
StorageStats stats = GetStorageStats();
bool over_usage_threshold = stats.in_use_bytes > threshold_bytes;
- bool force_gc = over_usage_threshold || (update_status.size() > 0);
+ bool heavy = (maintenance_type == MaintenanceType::kHeavy);
+ bool force_gc = heavy || over_usage_threshold || (update_status.size() > 0);
// TODO: look in to making an iterator method for cycling through sectors
// starting from last_new_sector_.
@@ -982,6 +984,7 @@
SectorDescriptor& sector_to_gc,
std::span<const Address> reserved_addresses) {
DBG(" Garbage Collect sector %u", sectors_.Index(sector_to_gc));
+
// Step 1: Move any valid entries in the GC sector to other sectors
if (sector_to_gc.valid_bytes() != 0) {
for (EntryMetadata& metadata : entry_cache_) {
@@ -998,9 +1001,12 @@
}
// Step 2: Reinitialize the sector
- sector_to_gc.mark_corrupt();
- PW_TRY(partition_.Erase(sectors_.BaseAddress(sector_to_gc), 1));
- sector_to_gc.set_writable_bytes(partition_.sector_size_bytes());
+ if (!sector_to_gc.Empty(partition_.sector_size_bytes())) {
+ sector_to_gc.mark_corrupt();
+ internal_stats_.sector_erase_count++;
+ PW_TRY(partition_.Erase(sectors_.BaseAddress(sector_to_gc), 1));
+ sector_to_gc.set_writable_bytes(partition_.sector_size_bytes());
+ }
DBG(" Garbage Collect sector %u complete", sectors_.Index(sector_to_gc));
return Status::Ok();
@@ -1098,7 +1104,7 @@
DBG(" Found sector %u with corruption", sectors_.Index(sector));
Status sector_status = GarbageCollectSector(sector, {});
if (sector_status.ok()) {
- error_stats_.corrupt_sectors_recovered += 1;
+ internal_stats_.corrupt_sectors_recovered += 1;
} else if (repair_status.ok() ||
repair_status == Status::ResourceExhausted()) {
repair_status = sector_status;
@@ -1156,7 +1162,7 @@
unsigned(redundancy()));
Status fill_status = AddRedundantEntries(metadata);
if (fill_status.ok()) {
- error_stats_.missing_redundant_entries_recovered += 1;
+ internal_stats_.missing_redundant_entries_recovered += 1;
DBG(" Key missing copies added");
} else {
DBG(" Failed to add key missing copies");
diff --git a/pw_kvs/key_value_store_test.cc b/pw_kvs/key_value_store_test.cc
index 2820793..140d174 100644
--- a/pw_kvs/key_value_store_test.cc
+++ b/pw_kvs/key_value_store_test.cc
@@ -439,6 +439,35 @@
EXPECT_EQ(kvs_.size(), 1u);
}
+TEST_F(LargeEmptyInitializedKvs, FullMaintenance) {
+ const uint8_t kValue1 = 0xDA;
+ const uint8_t kValue2 = 0x12;
+
+ // Write a key and write again with a different value, resulting in a stale
+ // entry from the first write.
+ ASSERT_EQ(Status::Ok(), kvs_.Put(keys[0], kValue1));
+ ASSERT_EQ(Status::Ok(), kvs_.Put(keys[0], kValue2));
+ EXPECT_EQ(kvs_.size(), 1u);
+
+ KeyValueStore::StorageStats stats = kvs_.GetStorageStats();
+ EXPECT_EQ(stats.sector_erase_count, 0u);
+ EXPECT_GT(stats.reclaimable_bytes, 0u);
+
+ // Do regular FullMaintenance, which should not touch the sector with valid
+ // data.
+ EXPECT_EQ(Status::Ok(), kvs_.FullMaintenance());
+ stats = kvs_.GetStorageStats();
+ EXPECT_EQ(stats.sector_erase_count, 0u);
+ EXPECT_GT(stats.reclaimable_bytes, 0u);
+
+ // Do aggressive FullMaintenance, which should GC the sector with valid data,
+ // resulting in no reclaimable bytes and an erased sector.
+ EXPECT_EQ(Status::Ok(), kvs_.HeavyMaintenance());
+ stats = kvs_.GetStorageStats();
+ EXPECT_EQ(stats.sector_erase_count, 1u);
+ EXPECT_EQ(stats.reclaimable_bytes, 0u);
+}
+
TEST(InMemoryKvs, Put_MaxValueSize) {
// Create and erase the fake flash.
Flash flash;
diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index 6988d6f..de4249e 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -180,10 +180,26 @@
StatusWithSize ValueSize(std::string_view key) const;
// Perform all maintenance possible, including all neeeded repairing of
- // corruption and garbage collection of all reclaimable space in the KVS. When
- // configured for manual recovery, this is the only way KVS repair is
- // triggered.
- Status FullMaintenance();
+ // corruption and garbage collection of reclaimable space in the KVS. When
+ // configured for manual recovery, this (along with FullMaintenance) is the
+ // only way KVS repair is triggered.
+ //
+ // - Heavy garbage collection of all reclaimable space, regardless of valid
+ // data in the sector.
+ Status HeavyMaintenance() {
+ return FullMaintenanceHelper(MaintenanceType::kHeavy);
+ }
+
+ // Perform all maintenance possible, including all neeeded repairing of
+ // corruption and garbage collection of reclaimable space in the KVS. When
+ // configured for manual recovery, this (along with HeavyMaintenance) is the
+ // only way KVS repair is triggered.
+ //
+ // - Regular will not garbage collect sectors with valid data unless the KVS
+ // is mostly full.
+ Status FullMaintenance() {
+ return FullMaintenanceHelper(MaintenanceType::kRegular);
+ }
// Perform a portion of KVS maintenance. If configured for at least lazy
// recovery, will do any needed repairing of corruption. Does garbage
@@ -292,6 +308,7 @@
size_t writable_bytes;
size_t in_use_bytes;
size_t reclaimable_bytes;
+ size_t sector_erase_count;
size_t corrupt_sectors_recovered;
size_t missing_redundant_entries_recovered;
};
@@ -445,6 +462,21 @@
KeyValueStore::Address& address,
std::span<const Address> addresses_to_skip);
+ // Perform all maintenance possible, including all neeeded repairing of
+ // corruption and garbage collection of reclaimable space in the KVS. When
+ // configured for manual recovery, this is the only way KVS repair is
+ // triggered.
+ //
+ // - Regular will not garbage collect sectors with valid data unless the KVS
+ // is mostly full.
+ // - Heavy will garbage collect all reclaimable space regardless of valid data
+ // in the sector.
+ enum class MaintenanceType {
+ kRegular,
+ kHeavy,
+ };
+ Status FullMaintenanceHelper(MaintenanceType maintenance_type);
+
// Find and garbage collect a singe sector that does not include an address to
// skip.
Status GarbageCollect(std::span<const Address> addresses_to_skip);
@@ -519,11 +551,12 @@
// make it mutable.
mutable bool error_detected_;
- struct ErrorStats {
+ struct InternalStats {
+ size_t sector_erase_count;
size_t corrupt_sectors_recovered;
size_t missing_redundant_entries_recovered;
};
- ErrorStats error_stats_;
+ InternalStats internal_stats_;
uint32_t last_transaction_id_;
};
