diff --git a/src/blockstore/blockstore_disk.cpp b/src/blockstore/blockstore_disk.cpp index df1f3bd6..a382f1ed 100644 --- a/src/blockstore/blockstore_disk.cpp +++ b/src/blockstore/blockstore_disk.cpp @@ -96,6 +96,7 @@ void blockstore_disk_t::parse_config(std::map & config (config["discard_on_start"] == "true" || config["discard_on_start"] == "1" || config["discard_on_start"] == "yes"); gc_on_start = config.find("gc_on_start") == config.end() || (config["gc_on_start"] == "true" || config["gc_on_start"] == "1" || config["gc_on_start"] == "yes"); + skip_double_claim = (config["skip_double_claim"] == "true" || config["skip_double_claim"] == "1" || config["skip_double_claim"] == "yes"); min_discard_size = parse_size(config["min_discard_size"]); if (!min_discard_size) min_discard_size = 1024*1024; diff --git a/src/blockstore/blockstore_disk.h b/src/blockstore/blockstore_disk.h index ba67f0ac..bae04cee 100644 --- a/src/blockstore/blockstore_disk.h +++ b/src/blockstore/blockstore_disk.h @@ -59,6 +59,8 @@ struct blockstore_disk_t bool discard_on_start = false; // GC on start (new store) bool gc_on_start = true; + // Skip double claim conflicts on start (new store, temporary until the bug is found) + bool skip_double_claim = false; uint64_t min_discard_size = 1024*1024; uint64_t discard_granularity = 0; diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index e5f53808..459faf08 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -408,6 +408,13 @@ corrupted_object: wr->inode, wr->stripe, wr->version, wr->big_intent().offset, wr->big_intent().len); goto corrupted_object; } + if ((wr->type() == BS_HEAP_BIG_INTENT || wr->type() == BS_HEAP_BIG_WRITE) && + wr->big().block_num >= dsk->block_count) + { + fprintf(stderr, "Error: big_write or big_intent entry %jx:%jx v%ju block_num is too large: %u > %lu. Metadata is incompatible with current parameters. ", + wr->inode, wr->stripe, wr->version, wr->big_intent().block_num, dsk->block_count); + goto corrupted_object; + } handle_write(block_num, wr); block_offset += wr->size; } @@ -593,6 +600,11 @@ void blockstore_heap_t::fill_recheck_queue() int blockstore_heap_t::mark_used_blocks() { int res = 0; + std::vector used_by; + if (dsk->skip_double_claim) + { + used_by.resize(dsk->block_count); + } for (auto & pgp: block_index) { for (auto & ip: pgp.second) @@ -646,10 +658,34 @@ int blockstore_heap_t::mark_used_blocks() { if (is_data_used(wr->big_location(this))) { - fprintf(stderr, "Error: double-claimed data block %u, second time by %jx:%jx l%ju\n", - wr->big().block_num, wr->inode, wr->stripe, wr->lsn); - res = EDOM; - return; + if (dsk->skip_double_claim) + { + // There is a BUG currently: + // Sometimes (under unknown conditions) deletion entries are removed from the disk + // earlier than previous big_writes. + // Until it's fixed, we provide a way to ignore such objects on start. + auto prev_li = used_by[wr->big().block_num]; + assert(prev_li); + // Newer LSN must be trusted. Remove the older object. + fprintf(stderr, "Block %u is double-claimed by entries %jx:%jx l%ju and %jx:%jx l%ju\n", + wr->big().block_num, prev_li->entry.inode, prev_li->entry.stripe, prev_li->entry.lsn, wr->inode, wr->stripe, wr->lsn); + if (init_erase_double_claim(prev_li, li)) + { + return; + } + } + else + { + fprintf(stderr, "Error: double-claimed data block %u, second time by %jx:%jx l%ju\n", + wr->big().block_num, wr->inode, wr->stripe, wr->lsn); + res = EDOM; + return; + } + } + if (dsk->skip_double_claim) + { + // Record the object which uses the data block + used_by[wr->big().block_num] = li; } use_data(wr->inode, wr->big_location(this)); } @@ -673,6 +709,116 @@ int blockstore_heap_t::mark_used_blocks() return res; } +void blockstore_heap_t::init_free_bad_entry(heap_entry_t *wr) +{ + if (wr->type() == BS_HEAP_SMALL_WRITE) + { + free_buffer_area(wr->inode, wr->small().location, wr->small().len); + } + else if (wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_BIG_INTENT) + { + free_data(wr->inode, wr->big_location(this)); + } +} + +void blockstore_heap_t::init_erase_bad_entry(heap_list_item_t *li) +{ + modify_alloc(li->block_num, [&](heap_block_info_t & inf) + { + for (size_t i = 0; i < inf.entries.size(); i++) + { + if (inf.entries[i] == li) + { + inf.entries.erase(inf.entries.begin()+i); + break; + } + } + inf.used_space -= li->entry.size; + inf.garbage_space -= (li->entry.is_garbage() ? li->entry.size : 0); + }); + recheck_modified_blocks.insert(li->block_num); + unlink_list_item(li); +} + +bool blockstore_heap_t::init_erase_double_claim(heap_list_item_t *prev_li, heap_list_item_t *cur_li) +{ + bool erase_prev = false; + bool erase_cur = false; + if (prev_li->entry.lsn < cur_li->entry.lsn) + { + erase_prev = true; + auto latest_li = prev_li; + while (latest_li->next) + { + latest_li = latest_li->next; + } + if (latest_li->entry.lsn >= cur_li->entry.lsn) + { + // LSN ranges intersect, erase both + erase_cur = true; + } + } + else + { + erase_cur = true; + auto latest_li = cur_li; + while (latest_li->next) + { + latest_li = latest_li->next; + } + if ((latest_li->entry.inode != prev_li->entry.inode || + latest_li->entry.stripe != prev_li->entry.stripe) && + latest_li->entry.lsn >= prev_li->entry.lsn) + { + // LSN ranges intersect, erase both + erase_prev = true; + } + } + if (erase_prev) + { + fprintf(stderr, "Erasing object %jx:%jx due to double-claim\n", prev_li->entry.inode, prev_li->entry.stripe); + auto erase_li = prev_li; + while (erase_li->next) + { + erase_li = erase_li->next; + } + bool overwritten = false; + while (erase_li) + { + auto prev_erase_li = erase_li->prev; + if (!overwritten) + { + init_free_bad_entry(&erase_li->entry); + overwritten = erase_li->entry.is_overwrite(); + } + init_erase_bad_entry(erase_li); + erase_li = prev_erase_li; + } + } + if (erase_cur) + { + fprintf(stderr, "Erasing object %jx:%jx due to double-claim\n", cur_li->entry.inode, cur_li->entry.stripe); + auto erase_li = cur_li->next; + while (erase_li) + { + // Only newer entries are marked as used + auto next_erase_li = erase_li->next; + init_free_bad_entry(&erase_li->entry); + init_erase_bad_entry(erase_li); + erase_li = next_erase_li; + } + erase_li = cur_li; + // Older ones are not + while (erase_li) + { + auto prev_erase_li = erase_li->prev; + init_erase_bad_entry(erase_li); + erase_li = prev_erase_li; + } + } + return erase_cur; +} + void blockstore_heap_t::recheck_full_gc() { uint32_t block_num = 0; @@ -2390,6 +2536,22 @@ void blockstore_heap_t::apply_inflight(heap_inflight_lsn_t & inflight) } void blockstore_heap_t::remove_list_item(heap_list_item_t *li) +{ + if (!li->next) + { + // The last freed entry must be a deletion + assert(!li->prev); + assert(li->entry.entry_type == BS_HEAP_DELETE|BS_HEAP_STABLE); + } + else if (!li->prev && li->next->entry.entry_type == (BS_HEAP_DELETE|BS_HEAP_STABLE)) + { + // free BS_HEAP_DELETEs when all previous entries are also freed + mark_garbage(li->next->block_num, &li->next->entry, UINT32_MAX); + } + unlink_list_item(li); +} + +void blockstore_heap_t::unlink_list_item(heap_list_item_t *li) { auto prev = li->prev; auto next = li->next; @@ -2399,25 +2561,20 @@ void blockstore_heap_t::remove_list_item(heap_list_item_t *li) } if (!next) { - // The last freed entry must be a deletion - assert(!prev); auto wr = &li->entry; - assert(wr->entry_type == BS_HEAP_DELETE|BS_HEAP_STABLE); auto & pg_idx = block_index[get_pg_id(wr->inode, wr->stripe)]; auto & inode_idx = pg_idx[wr->inode]; heap_inode_map_t::iterator li_it; heap_list_item_t *old_li = NULL; inode_map_get(inode_idx, li_it, old_li, wr->stripe); - inode_map_erase(pg_idx, inode_idx, li_it, old_li); + if (!prev) + inode_map_erase(pg_idx, inode_idx, li_it, old_li); + else + inode_map_replace(inode_idx, li_it, prev); } else { next->prev = prev; - if (!prev && next->entry.entry_type == (BS_HEAP_DELETE|BS_HEAP_STABLE)) - { - // free BS_HEAP_DELETEs when all previous entries are also freed - mark_garbage(next->block_num, &next->entry, UINT32_MAX); - } } if (li->entry.is_garbage()) { diff --git a/src/blockstore/blockstore_heap.h b/src/blockstore/blockstore_heap.h index 2cc26ff3..21852762 100644 --- a/src/blockstore/blockstore_heap.h +++ b/src/blockstore/blockstore_heap.h @@ -220,6 +220,9 @@ class blockstore_heap_t bool validate_object(heap_entry_t *obj); void fill_recheck_queue(); int mark_used_blocks(); + void init_free_bad_entry(heap_entry_t *wr); + void init_erase_bad_entry(heap_list_item_t *li); + bool init_erase_double_claim(heap_list_item_t *prev_li, heap_list_item_t *cur_li); void recheck_full_gc(); void recheck_buffer(heap_entry_t *cwr, uint8_t *buf); void defragment_block(uint32_t block_num); @@ -229,6 +232,7 @@ class blockstore_heap_t int allocate_entry(uint32_t entry_size, uint32_t *block_num, bool allow_last_free); void insert_list_item(heap_list_item_t *li); void remove_list_item(heap_list_item_t *li); + void unlink_list_item(heap_list_item_t *li); int add_entry(uint32_t wr_size, uint32_t *modified_block, bool allow_last_free, bool explicit_complete, std::function fill_entry); int add_simple(heap_entry_t *obj, uint64_t version, uint32_t *modified_block, uint32_t entry_type); diff --git a/src/test/test_heap.cpp b/src/test/test_heap.cpp index 706501fe..0ba57394 100644 --- a/src/test/test_heap.cpp +++ b/src/test/test_heap.cpp @@ -20,7 +20,7 @@ static int count_writes(blockstore_heap_t & heap, heap_entry_t *obj) return n; } -#define FREE_SPACE_BIT 0x8000 +#define BS_HEAP_FREE_SPACE 0xAB8F #define GARBAGE_BIT ((uint64_t)1 << 63) bool check_used_space(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint32_t block_num) @@ -30,10 +30,14 @@ bool check_used_space(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint32_ uint8_t *data = buf; uint8_t *end = data+dsk.meta_block_size; uint32_t used = 0; - while (data < end) + while (data <= end-4) { heap_entry_t *wr = ((heap_entry_t*)data); - if (!(wr->size & FREE_SPACE_BIT) && !wr->is_garbage()) + if (wr->entry_type == BS_HEAP_FREE_SPACE) + { + break; + } + if (!wr->is_garbage()) { used += wr->size; } @@ -41,7 +45,7 @@ bool check_used_space(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint32_ { break; } - data += (wr->size & ~FREE_SPACE_BIT); + data += wr->size; } free(buf); return used == heap.get_meta_block_used_space(block_num); @@ -2147,6 +2151,193 @@ void test_explicit_complete() printf("OK test_explicit_complete\n"); } +void test_skip_double_claim() +{ + blockstore_disk_t dsk; + _test_init(dsk, false); + dsk.skip_double_claim = true; + std::vector tmp(dsk.meta_block_size); + std::vector out(dsk.meta_block_size*3); + std::vector buffer_area(dsk.journal_device_size); + heap_entry_t *wr1 = NULL, *wr2 = NULL, *wr3 = NULL, *wr4 = NULL; + uint32_t total_size = 0; + + { + blockstore_heap_t heap(&dsk, buffer_area.data()); + + wr1 = (heap_entry_t*)(tmp.data() + total_size); + wr1->size = heap.get_big_entry_size(); + wr1->entry_type = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; + wr1->lsn = 1; + wr1->inode = INODE_WITH_POOL(1, 1); + wr1->stripe = 0; + wr1->version = 1; + wr1->set_big_location(&heap, 0x40000); // <-- overwritten + wr1->crc32c = wr1->calc_crc32c(); + total_size += wr1->size; + + wr2 = (heap_entry_t*)(tmp.data() + total_size); + wr2->size = heap.get_big_entry_size(); + wr2->entry_type = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; + wr2->lsn = 2; + wr2->inode = INODE_WITH_POOL(1, 1); + wr2->stripe = 0; + wr2->version = 2; + wr2->set_big_location(&heap, 0); // <-- double claimed + wr2->crc32c = wr2->calc_crc32c(); + total_size += wr2->size; + + wr3 = (heap_entry_t*)(tmp.data() + total_size); + wr3->size = heap.get_big_entry_size(); + wr3->entry_type = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; + wr3->lsn = 3; + wr3->inode = INODE_WITH_POOL(1, 1); + wr3->stripe = 0x20000; + wr3->version = 1; + wr3->set_big_location(&heap, 0); // <-- double claimed + wr3->crc32c = wr3->calc_crc32c(); + total_size += wr3->size; + + wr4 = (heap_entry_t*)(tmp.data() + total_size); + wr4->size = heap.get_big_entry_size(); + wr4->entry_type = BS_HEAP_BIG_WRITE; // <-- unstable + wr4->lsn = 4; + wr4->inode = INODE_WITH_POOL(1, 1); + wr4->stripe = 0x20000; + wr4->version = 2; + wr4->set_big_location(&heap, 0x20000); + wr4->crc32c = wr4->calc_crc32c(); + total_size += wr4->size; + + *(uint16_t*)(tmp.data() + total_size) = dsk.meta_block_size - total_size; + *(uint16_t*)(tmp.data() + total_size + 2) = BS_HEAP_FREE_SPACE; + + uint64_t entries_loaded; + heap.load_blocks(0, dsk.meta_block_size, tmp.data(), false, entries_loaded); + heap.finish_load(); + bool done = heap.recheck_small_writes([&](bool, uint64_t, uint64_t, uint8_t*, std::function cb) {}, 1); + assert(done); + heap.finish_recheck(); + auto mod = heap.get_recheck_modified_blocks(); + assert(mod.size() == 1); + assert(mod[0] == 0); + + // [1 2] [3 4] - should erase first + + object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + heap_entry_t *obj = heap.read_entry(oid); + assert(!obj); + + oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0x20000 }; + obj = heap.read_entry(oid); + assert(obj); + + assert(heap.is_data_used(0)); + assert(heap.is_data_used(0x20000)); + assert(!heap.is_data_used(0x40000)); + + assert(check_used_space(heap, dsk, 0)); + + heap.get_meta_block(0, out.data()); + } + + { + blockstore_heap_t heap(&dsk, buffer_area.data()); + + wr1->lsn = 1; + wr1->crc32c = wr1->calc_crc32c(); + wr2->lsn = 3; + wr2->crc32c = wr2->calc_crc32c(); + wr3->lsn = 2; + wr3->crc32c = wr3->calc_crc32c(); + wr4->lsn = 4; + wr4->crc32c = wr4->calc_crc32c(); + + uint64_t entries_loaded; + heap.load_blocks(0, dsk.meta_block_size, tmp.data(), false, entries_loaded); + heap.finish_load(); + bool done = heap.recheck_small_writes([&](bool, uint64_t, uint64_t, uint8_t*, std::function cb) {}, 1); + assert(done); + heap.finish_recheck(); + auto mod = heap.get_recheck_modified_blocks(); + assert(mod.size() == 1); + assert(mod[0] == 0); + + // [1 [2 3] 4] - intersect - should erase both + + object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + heap_entry_t *obj = heap.read_entry(oid); + assert(!obj); + + oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0x20000 }; + obj = heap.read_entry(oid); + assert(!obj); + + assert(!heap.is_data_used(0)); + assert(!heap.is_data_used(0x20000)); + assert(!heap.is_data_used(0x40000)); + + assert(check_used_space(heap, dsk, 0)); + + heap.get_meta_block(0, out.data()+dsk.meta_block_size); + } + + { + blockstore_heap_t heap(&dsk, buffer_area.data()); + + // [3 4] [1 2] - should erase second + + wr1->lsn = 3; + wr1->crc32c = wr1->calc_crc32c(); + wr2->lsn = 4; + wr2->crc32c = wr2->calc_crc32c(); + wr3->lsn = 1; + wr3->crc32c = wr3->calc_crc32c(); + wr4->lsn = 2; + wr4->crc32c = wr4->calc_crc32c(); + + uint64_t entries_loaded; + heap.load_blocks(0, dsk.meta_block_size, tmp.data(), false, entries_loaded); + heap.finish_load(); + bool done = heap.recheck_small_writes([&](bool, uint64_t, uint64_t, uint8_t*, std::function cb) {}, 1); + assert(done); + heap.finish_recheck(); + auto mod = heap.get_recheck_modified_blocks(); + assert(mod.size() == 1); + assert(mod[0] == 0); + + object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + heap_entry_t *obj = heap.read_entry(oid); + assert(obj); + + oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0x20000 }; + obj = heap.read_entry(oid); + assert(!obj); + + assert(heap.is_data_used(0)); + assert(!heap.is_data_used(0x20000)); + assert(!heap.is_data_used(0x40000)); + + assert(check_used_space(heap, dsk, 0)); + + heap.get_meta_block(0, out.data()+dsk.meta_block_size*2); + } + + // Validate persisted variants + for (int i = 0; i < 3; i++) + { + blockstore_heap_t heap(&dsk, buffer_area.data()); + uint64_t entries_loaded; + heap.load_blocks(0, dsk.meta_block_size, out.data() + dsk.meta_block_size*i, false, entries_loaded); + heap.finish_load(); + bool done = heap.recheck_small_writes([&](bool, uint64_t, uint64_t, uint8_t*, std::function cb) {}, 1); + assert(done); + heap.finish_recheck(); + auto mod = heap.get_recheck_modified_blocks(); + assert(mod.size() == 0); + } +} + // FIXME: Add a test for big_intent, incl. explicit_complete with big_intent over big_write over deletion over big_write :) int main(int narg, char *args[]) @@ -2187,5 +2378,6 @@ int main(int narg, char *args[]) test_recalc_stats(); test_redirect_intent_csums(); test_explicit_complete(); + test_skip_double_claim(); return 0; }