Skip to content

TDBStore: Perform garbage collection on failed writes #9200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 8, 2019
Merged

TDBStore: Perform garbage collection on failed writes #9200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,7 @@ static void set_several_keys_multithreaded()
for (i = 0; i < num_of_threads; i++) {
res = kvstore->get(keys[i], buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

TEST_ASSERT_EQUAL_STRING_LEN(buffer, data, data_size);

}

for (i = 0; i < num_of_threads; i++) {
Expand Down Expand Up @@ -329,6 +327,7 @@ static void set_key_value_two_byte_size()
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_two, 1);
memset(buffer, 0, buffer_size);

Expand All @@ -346,6 +345,7 @@ static void set_key_value_five_byte_size()
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_five, 4);
memset(buffer, 0, buffer_size);

Expand All @@ -363,6 +363,7 @@ static void set_key_value_fifteen_byte_size()
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 14);
memset(buffer, 0, buffer_size);

Expand All @@ -380,6 +381,7 @@ static void set_key_value_seventeen_byte_size()
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 16);
memset(buffer, 0, buffer_size);

Expand Down Expand Up @@ -451,7 +453,8 @@ static void set_key_init_deinit()
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);

res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
TEST_ASSERT_EQUAL_STRING(buffer, data);
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
TEST_ASSERT_EQUAL_STRING(data, buffer);
memset(buffer, 0, buffer_size);

res = kvstore->remove(key);
Expand Down
34 changes: 32 additions & 2 deletions features/storage/kvstore/tdbstore/TDBStore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data
uint32_t offset;
uint32_t hash, ram_table_ind;
inc_set_handle_t *ih;
bool need_gc = false;

if (!is_valid_key(key)) {
return MBED_ERROR_INVALID_ARGUMENT;
Expand Down Expand Up @@ -485,14 +486,19 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data
// Write key now
ret = write_area(_active_area, ih->bd_curr_offset, ih->header.key_size, key);
if (ret) {
need_gc = true;
goto fail;
}
ih->bd_curr_offset += ih->header.key_size;
goto end;

fail:
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
garbage_collection();
}
// mark handle as invalid by clearing magic field in header
ih->header.magic = 0;

_mutex.unlock();

end:
Expand All @@ -503,6 +509,7 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d
{
int ret = MBED_SUCCESS;
inc_set_handle_t *ih;
bool need_gc = false;

if (handle != _inc_set_handle) {
return MBED_ERROR_INVALID_ARGUMENT;
Expand Down Expand Up @@ -532,12 +539,17 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d
// Write the data chunk
ret = write_area(_active_area, ih->bd_curr_offset, data_size, value_data);
if (ret) {
need_gc = true;
goto end;
}
ih->bd_curr_offset += data_size;
ih->offset_in_data += data_size;

end:
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
garbage_collection();
}

_inc_set_mutex.unlock();
return ret;
}
Expand All @@ -548,6 +560,8 @@ int TDBStore::set_finalize(set_handle_t handle)
inc_set_handle_t *ih;
ram_table_entry_t *ram_table = (ram_table_entry_t *) _ram_table;
ram_table_entry_t *entry;
bool need_gc = false;
uint32_t actual_data_size, hash, flags, next_offset;

if (handle != _inc_set_handle) {
return MBED_ERROR_INVALID_ARGUMENT;
Expand All @@ -563,21 +577,22 @@ int TDBStore::set_finalize(set_handle_t handle)

if (ih->offset_in_data != ih->header.data_size) {
ret = MBED_ERROR_INVALID_SIZE;
// Need GC as otherwise our storage is left in a non-usable state
garbage_collection();
need_gc = true;
goto end;
}

// Write header
ret = write_area(_active_area, ih->bd_base_offset, sizeof(record_header_t), &ih->header);
if (ret) {
need_gc = true;
goto end;
}

// Need to flush buffered BD as our record is totally written now
os_ret = _buff_bd->sync();
if (os_ret) {
ret = MBED_ERROR_WRITE_FAILED;
need_gc = true;
goto end;
}

Expand All @@ -586,6 +601,16 @@ int TDBStore::set_finalize(set_handle_t handle)
goto end;
}

// Writes may fail without returning a failure (specially in flash components). Reread the record
// to ensure write success (this won't read the data anywhere - just use the CRC calculation).
ret = read_record(_active_area, ih->bd_base_offset, 0, 0, (uint32_t) -1,
actual_data_size, 0, false, false, false, false,
hash, flags, next_offset);
if (ret) {
need_gc = true;
goto end;
}

// Update RAM table
if (ih->header.flags & delete_flag) {
_num_keys--;
Expand All @@ -611,10 +636,15 @@ int TDBStore::set_finalize(set_handle_t handle)
_free_space_offset = align_up(ih->bd_curr_offset, _prog_size);

end:
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
garbage_collection();
}

// mark handle as invalid by clearing magic field in header
ih->header.magic = 0;

_inc_set_mutex.unlock();

if (ih->bd_base_offset != _master_record_offset) {
_mutex.unlock();
}
Expand Down