From 3eecf9048c5bda4e3c03dd2de54a7ce15bf14342 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sun, 26 Oct 2025 15:20:46 +0300 Subject: [PATCH] Simplify compaction --- src/blockstore/blockstore_flush.cpp | 119 ++++++++-------- src/blockstore/blockstore_flush.h | 7 +- src/blockstore/blockstore_heap.cpp | 201 ++++++++-------------------- src/blockstore/blockstore_heap.h | 10 +- src/blockstore/blockstore_read.cpp | 20 +-- src/test/test_heap.cpp | 42 ++++-- 6 files changed, 155 insertions(+), 244 deletions(-) diff --git a/src/blockstore/blockstore_flush.cpp b/src/blockstore/blockstore_flush.cpp index 29c7411c..5ade09fe 100644 --- a/src/blockstore/blockstore_flush.cpp +++ b/src/blockstore/blockstore_flush.cpp @@ -24,10 +24,12 @@ journal_flusher_t::journal_flusher_t(blockstore_impl_t *bs) { co[i].co_id = i; co[i].bs = bs; - if (bs->dsk.csum_block_size > bs->dsk.bitmap_granularity) + co[i].new_bmp = (uint8_t*)malloc_or_die(3*bs->dsk.clean_entry_bitmap_size); + co[i].new_ext_bmp = co[i].new_bmp + bs->dsk.clean_entry_bitmap_size; + co[i].punch_bmp = co[i].new_bmp + 2*bs->dsk.clean_entry_bitmap_size; + if (bs->dsk.csum_block_size > 0) { co[i].new_csums = (uint8_t*)malloc_or_die(bs->dsk.data_block_size / bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); - co[i].new_bmp = (uint8_t*)malloc_or_die(bs->dsk.clean_entry_bitmap_size); } co[i].flusher = this; } @@ -69,6 +71,8 @@ journal_flusher_co::~journal_flusher_co() free(new_bmp); new_bmp = NULL; } + new_ext_bmp = NULL; + punch_bmp = NULL; free_buffers(); } @@ -218,8 +222,21 @@ resume_1: free_buffers(); copy_count = 0; fsynced_lsn = bs->heap->get_fsynced_lsn(); + bitmap_copied = false; + memset(new_bmp, 0, bs->dsk.clean_entry_bitmap_size); + csum_copy.clear(); compact_info = bs->heap->iterate_compaction(cur_obj, fsynced_lsn, flusher->force_start, [&](heap_entry_t *wr) { + if (!bitmap_copied) + { + memcpy(new_ext_bmp, wr->get_ext_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); + bitmap_copied = true; + } + bitmap_set(new_bmp, wr->small().offset, wr->small().len, bs->dsk.bitmap_granularity); + if (bs->dsk.csum_block_size && bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity) + { + csum_copy.push_back(wr); + } if (wr->type() == BS_HEAP_SMALL_WRITE || wr->type() == BS_HEAP_INTENT_WRITE && bs->dsk.csum_block_size > bs->dsk.bitmap_granularity) { @@ -234,10 +251,28 @@ resume_1: bs->heap->unlock_entry(cur_oid); goto resume_0; } + mem_or(new_bmp, compact_info.clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); + if (!bitmap_copied) + { + memcpy(new_ext_bmp, compact_info.clean_wr->get_ext_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); + bitmap_copied = true; + } + if (bs->dsk.csum_block_size && bs->dsk.csum_block_size <= bs->dsk.bitmap_granularity) + { + memcpy(new_csums, compact_info.clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); + for (size_t i = csum_copy.size(); i > 0; i--) + { + auto wr = csum_copy[i-1]; + memcpy(new_csums + wr->small().offset/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF), + wr->get_checksums(bs->heap), wr->small().len/bs->dsk.csum_block_size*(bs->dsk.data_csum_type & 0xFF)); + } + csum_copy.clear(); + } + clean_loc = compact_info.clean_wr->big_location(bs->heap); flusher->active_flushers++; if (bs->log_level > 10) { - printf("Compacting %jx:%jx l%ju .. l%ju\n", cur_oid.inode, cur_oid.stripe, compact_info.clean_lsn, compact_info.compact_lsn); + printf("Compacting %jx:%jx l%ju .. l%ju\n", cur_oid.inode, cur_oid.stripe, compact_info.clean_wr->lsn, compact_info.compact_lsn); } overwrite_start = overwrite_end = 0; if (read_vec.size() > 0) @@ -286,7 +321,7 @@ resume_3: { resume_4: modified_block = UINT32_MAX; - res = bs->heap->add_punch_holes(cur_obj, compact_info.clean_lsn, compact_info.clean_version, new_bmp, new_csums, &modified_block); + res = bs->heap->punch_holes(compact_info.clean_wr, punch_bmp, new_csums, &modified_block); if (res == ENOENT) { // Abort compaction @@ -330,7 +365,7 @@ resume_9: data->iov = (struct iovec){ read_vec[i].buf + (read_vec[i].copy_flags & COPY_BUF_PADDED ? read_vec[i].offset - read_vec[i].disk_offset : 0), (size_t)read_vec[i].len }; data->callback = simple_callback_w; - io_uring_prep_writev(sqe, bs->dsk.data_fd, &data->iov, 1, bs->dsk.data_offset + compact_info.clean_loc + read_vec[i].offset); + io_uring_prep_writev(sqe, bs->dsk.data_fd, &data->iov, 1, bs->dsk.data_offset + clean_loc + read_vec[i].offset); wait_count++; } } @@ -358,7 +393,8 @@ resume_11: flusher->flushing.erase(cur_oid); goto resume_0; } - bs->heap->add_compact(cur_obj, compact_info.compact_lsn, &modified_block, new_csums); + bs->heap->add_compact(cur_obj, compact_info.compact_version, compact_info.compact_lsn, clean_loc, + compact_info.do_delete, &modified_block, new_bmp, new_ext_bmp, new_csums); resume_12: resume_13: if (!write_meta_block(12)) @@ -414,11 +450,11 @@ void journal_flusher_co::fill_partial_checksum_blocks() .copy_flags = COPY_BUF_DATA | copy_flags, .offset = blk_begin, .len = blk_end - blk_begin, - .disk_loc = compact_info.clean_loc, + .disk_loc = clean_loc, .disk_offset = blk_begin, .disk_len = blk_end - blk_begin, .buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, blk_end - blk_begin), - .wr_lsn = compact_info.clean_lsn, + .wr = compact_info.clean_wr, }); } auto & vec = read_vec[read_vec.size()-1]; @@ -468,22 +504,16 @@ int journal_flusher_co::check_and_punch_checksums() auto & vec = read_vec[i]; if (!(vec.copy_flags & (COPY_BUF_COALESCED|COPY_BUF_ZERO|COPY_BUF_SKIP_CSUM))) { - heap_entry_t *wr = cur_obj; - while (wr && wr->lsn != vec.wr_lsn) // FIXME: Skip compacted - { - wr = bs->heap->prev(wr); - } - assert(wr); - uint32_t *csums = (uint32_t*)(wr->get_checksums(bs->heap) + uint32_t *csums = (uint32_t*)(vec.wr->get_checksums(bs->heap) + (vec.disk_offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF) - - ((wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_BIG_INTENT) - ? 0 : (wr->small().offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF))); + - ((vec.wr->type() == BS_HEAP_BIG_WRITE || vec.wr->type() == BS_HEAP_BIG_INTENT) + ? 0 : (vec.wr->small().offset/bs->dsk.csum_block_size)*(bs->dsk.data_csum_type & 0xFF))); bs->heap->calc_block_checksums( - csums, vec.buf, wr->get_int_bitmap(bs->heap), vec.disk_offset, vec.disk_offset+vec.disk_len, false, + csums, vec.buf, vec.wr->get_int_bitmap(bs->heap), vec.disk_offset, vec.disk_offset+vec.disk_len, false, [&](uint32_t mismatch_pos, uint32_t expected_csum, uint32_t real_csum) { printf("Checksum mismatch during compaction in object %jx:%jx v%ju, offset 0x%x in %s area at offset 0x%jx: got %08x, expected %08x\n", - cur_oid.inode, cur_oid.stripe, wr->version, mismatch_pos, + cur_oid.inode, cur_oid.stripe, vec.wr->version, mismatch_pos, (vec.copy_flags & COPY_BUF_JOURNAL ? "buffer" : "data"), vec.disk_loc+vec.disk_offset, real_csum, expected_csum); csum_ok = false; @@ -502,24 +532,8 @@ int journal_flusher_co::check_and_punch_checksums() // Nothing to do return 0; } - heap_entry_t *clean_wr = NULL; - for (auto wr = cur_obj; wr; wr = bs->heap->prev(wr)) - { - if (wr->is_overwrite() && wr->lsn > compact_info.clean_lsn && - wr->lsn <= fsynced_lsn) - { - // Object is overwritten, abort compaction - return ENOENT; - } - if (wr->lsn == compact_info.clean_lsn) - { - clean_wr = wr; - break; - } - } - assert(clean_wr); - memcpy(new_bmp, clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); - memcpy(new_csums, clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); + memcpy(punch_bmp, compact_info.clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); + memcpy(new_csums, compact_info.clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); // Clear bits for (auto & vec: read_vec) { @@ -530,7 +544,7 @@ int journal_flusher_co::check_and_punch_checksums() if (!(vec.copy_flags & COPY_BUF_COALESCED) && ((vec.offset % bs->dsk.csum_block_size) || (vec.len % bs->dsk.csum_block_size))) { - bitmap_clear(new_bmp, vec.offset, vec.len, bs->dsk.bitmap_granularity); + bitmap_clear(punch_bmp, vec.offset, vec.len, bs->dsk.bitmap_granularity); } } // Update partial block checksums @@ -539,7 +553,7 @@ int journal_flusher_co::check_and_punch_checksums() if (vec.copy_flags & COPY_BUF_CSUM_FILL) { uint32_t csum_off = vec.offset/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF); - bs->heap->calc_block_checksums((uint32_t*)(new_csums+csum_off), vec.buf, new_bmp, vec.offset, vec.offset+vec.len, true, NULL); + bs->heap->calc_block_checksums((uint32_t*)(new_csums+csum_off), vec.buf, punch_bmp, vec.offset, vec.offset+vec.len, true, NULL); } } // Modified, we should add_punch_holes and then write the block to disk @@ -552,32 +566,7 @@ bool journal_flusher_co::calc_block_checksums() { return true; } - heap_entry_t *clean_wr = NULL; - for (auto wr = cur_obj; wr; wr = bs->heap->prev(wr)) - { - if (wr->is_overwrite() && wr->lsn > compact_info.clean_lsn && - wr->lsn <= fsynced_lsn) - { - // Object is overwritten, abort compaction - return false; - } - if (wr->lsn == compact_info.clean_lsn) - { - clean_wr = wr; - break; - } - } - assert(clean_wr); - memcpy(new_bmp, clean_wr->get_int_bitmap(bs->heap), bs->dsk.clean_entry_bitmap_size); - memcpy(new_csums, clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); - // Set bits - for (auto & vec: read_vec) - { - if (!(vec.copy_flags & (COPY_BUF_COALESCED|COPY_BUF_CSUM_FILL))) - { - bitmap_set(new_bmp, vec.offset, vec.len, bs->dsk.bitmap_granularity); - } - } + memcpy(new_csums, compact_info.clean_wr->get_checksums(bs->heap), bs->dsk.data_block_size/bs->dsk.csum_block_size * (bs->dsk.data_csum_type & 0xFF)); // Update block checksums size_t i = 0; while (i < read_vec.size() && !(read_vec[i].copy_flags & COPY_BUF_CSUM_FILL)) diff --git a/src/blockstore/blockstore_flush.h b/src/blockstore/blockstore_flush.h index e28ade08..a5d5b0e9 100644 --- a/src/blockstore/blockstore_flush.h +++ b/src/blockstore/blockstore_flush.h @@ -6,7 +6,7 @@ struct copy_buffer_t uint32_t copy_flags; uint64_t offset, len, disk_loc, disk_offset, disk_len; uint8_t *buf; - uint64_t wr_lsn; + heap_entry_t *wr; }; struct meta_sector_t @@ -38,6 +38,8 @@ class journal_flusher_co struct ring_data_t *data; uint8_t *new_csums = NULL; uint8_t *new_bmp = NULL; + uint8_t *punch_bmp = NULL; + uint8_t *new_ext_bmp = NULL; std::function simple_callback_r, simple_callback_w; @@ -45,10 +47,13 @@ class journal_flusher_co heap_entry_t *cur_obj; uint64_t fsynced_lsn; heap_compact_t compact_info; + uint64_t clean_loc; uint32_t modified_block; + bool bitmap_copied; bool should_repeat; std::vector read_vec; + std::vector csum_copy; uint32_t overwrite_start, overwrite_end; int i, res; bool read_to_fill_incomplete; diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index f2e934be..cbebf42a 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -1053,10 +1053,7 @@ int blockstore_heap_t::add_small_write(object_id oid, heap_entry_t *old_head, ui if (bitmap) memcpy(wr->get_ext_bitmap(this), bitmap, dsk->clean_entry_bitmap_size); else if (old_head) - { - old_head = read_entry(oid); memcpy(wr->get_ext_bitmap(this), old_head->get_ext_bitmap(this), dsk->clean_entry_bitmap_size); - } else memset(wr->get_ext_bitmap(this), 0, dsk->clean_entry_bitmap_size); calc_checksums(wr, (uint8_t*)data, true); @@ -1136,139 +1133,42 @@ int blockstore_heap_t::add_big_intent(object_id oid, heap_entry_t *old_head, uin }); } -int blockstore_heap_t::add_compact(heap_entry_t *obj, uint64_t to_lsn, uint32_t *modified_block, uint8_t *new_csums) +int blockstore_heap_t::add_compact(heap_entry_t *obj, uint64_t compact_version, uint64_t compact_lsn, uint64_t compact_location, + bool do_delete, uint32_t *modified_block, uint8_t *new_int_bitmap, uint8_t *new_ext_bitmap, uint8_t *new_csums) { - // Slightly tricky - we don't want to compact an object if it's overwritten or deleted during compaction + if (do_delete) { - heap_entry_t *old_wr = obj; - while (old_wr && !old_wr->is_overwrite()) + return add_entry(get_simple_entry_size(), modified_block, false, [&](heap_entry_t *wr) { - old_wr = prev(old_wr); - } - if (!old_wr) - { - // Check if we have to remove the object at all - bool has_entry = false; - iterate_with_stable(obj, obj->lsn, [&](heap_entry_t *old_wr, bool stable) - { - has_entry = true; - return false; - }); - if (!has_entry) - { - uint64_t compact_lsn = obj->lsn; - return add_entry(get_simple_entry_size(), modified_block, false, [&](heap_entry_t *wr) - { - wr->entry_type = BS_HEAP_DELETE|BS_HEAP_STABLE; - wr->inode = obj->inode; - wr->stripe = obj->stripe; - wr->version = 0; - wr->lsn = compact_lsn; - }); - } - } - else if (old_wr->lsn > to_lsn) - { - return ENOENT; - } + wr->entry_type = BS_HEAP_DELETE|BS_HEAP_STABLE; + wr->inode = obj->inode; + wr->stripe = obj->stripe; + wr->version = 0; + wr->lsn = compact_lsn; + }); } - auto oid = (object_id){ .inode = obj->inode, .stripe = obj->stripe }; uint32_t wr_size = get_big_entry_size(); return add_entry(wr_size, modified_block, true, [&](heap_entry_t *new_wr) { - // obj and old_wr are invalid, re-read them - the block could have been compacted - obj = read_entry(oid); - while (obj && obj->lsn > to_lsn) - { - // skip new entries - obj = prev(obj); - } - assert(obj); - new_wr->entry_type = BS_HEAP_BIG_WRITE | BS_HEAP_STABLE; + new_wr->entry_type = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; new_wr->inode = obj->inode; new_wr->stripe = obj->stripe; - memset(new_wr->get_int_bitmap(this), 0, dsk->clean_entry_bitmap_size); - bool need_copy = false, bitmap_copied = false; - std::vector cswr; - // Determine the latest compacted entry - uint64_t compact_lsn = obj->lsn, compact_version = obj->version; - iterate_with_stable(obj, to_lsn, [&](heap_entry_t *old_wr, bool stable) - { - if (!stable) - { - // This entry is still uncommitted, so it's not compacted and makes a gap - compact_lsn = old_wr->lsn-1; - compact_version = prev(old_wr)->version; - } - return !old_wr->is_overwrite(); - }); new_wr->version = compact_version; new_wr->lsn = compact_lsn; - bool found = false; - iterate_with_stable(obj, compact_lsn, [&](heap_entry_t *old_wr, bool stable) - { - if (!stable) - return true; - if (old_wr->type() == BS_HEAP_SMALL_WRITE || old_wr->type() == BS_HEAP_INTENT_WRITE) - { - if (!bitmap_copied) - { - memcpy(new_wr->get_ext_bitmap(this), old_wr->get_ext_bitmap(this), dsk->clean_entry_bitmap_size); - bitmap_copied = true; - } - bitmap_set(new_wr->get_int_bitmap(this), old_wr->small().offset, old_wr->small().len, dsk->bitmap_granularity); - if (dsk->data_csum_type && old_wr->small().len > 0) - { - if (dsk->csum_block_size == dsk->bitmap_granularity) - cswr.push_back(old_wr); - else - need_copy = true; - } - } - else if (old_wr->type() == BS_HEAP_BIG_WRITE) - { - found = true; - new_wr->big().block_num = old_wr->big().block_num; - mem_or(new_wr->get_int_bitmap(this), old_wr->get_int_bitmap(this), dsk->clean_entry_bitmap_size); - if (need_copy) - memcpy(new_wr->get_checksums(this), new_csums, dsk->data_block_size/dsk->csum_block_size*(dsk->data_csum_type & 0xFF)); - else if (dsk->data_csum_type) - { - // Copy checksums in the reverse order - memcpy(new_wr->get_checksums(this), old_wr->get_checksums(this), dsk->data_block_size/dsk->csum_block_size*(dsk->data_csum_type & 0xFF)); - for (size_t i = cswr.size(); i > 0; i--) - { - heap_entry_t *old_wr = cswr[i-1]; - memcpy(new_wr->get_checksums(this) + old_wr->small().offset/dsk->csum_block_size*(dsk->data_csum_type & 0xFF), - old_wr->get_checksums(this), old_wr->small().len/dsk->csum_block_size*(dsk->data_csum_type & 0xFF)); - } - } - return false; - } - return true; - }); - assert(found); + new_wr->set_big_location(this, compact_location); + memcpy(new_wr->get_int_bitmap(this), new_int_bitmap, dsk->clean_entry_bitmap_size); + memcpy(new_wr->get_ext_bitmap(this), new_ext_bitmap, dsk->clean_entry_bitmap_size); + if (dsk->data_csum_type && new_csums) + memcpy(new_wr->get_checksums(this), new_csums, dsk->data_block_size/dsk->csum_block_size*(dsk->data_csum_type & 0xFF)); }); } // A bit of a hack: overwrite the bitmap in an existing entry -int blockstore_heap_t::add_punch_holes(heap_entry_t *obj, uint64_t to_lsn, uint64_t version, uint8_t *new_bitmap, uint8_t *new_csums, uint32_t *modified_block) +int blockstore_heap_t::punch_holes(heap_entry_t *wr, uint8_t *new_bitmap, uint8_t *new_csums, uint32_t *modified_block) { assert(dsk->data_csum_type && dsk->csum_block_size > dsk->bitmap_granularity); assert(new_csums); - // Abort if the object is overwritten or deleted during compaction - heap_entry_t *wr = obj; - while (wr && wr->lsn != to_lsn && !wr->is_overwrite()) - { - wr = prev(wr); - } - if (!wr || wr->lsn > to_lsn) - { - return ENOENT; - } - auto & idx = block_index[get_pg_id(obj->inode, obj->stripe)][obj->inode][obj->stripe]; - assert(idx.ptr); - uint32_t block_num = idx.ptr->block_num; + uint32_t block_num = list_item(wr)->block_num; auto & inf = block_info.at(block_num); if (inf.is_writing) { @@ -1566,19 +1466,23 @@ void blockstore_heap_t::iterate_with_stable(heap_entry_t *obj, uint64_t max_lsn, } } +// Interesting cases: +// 1) BIG_STABLE(v1 l1) SMALL(v2 l2) SMALL(v3 l3) SMALL(v4 l4) ROLLBACK(v3 l5) COMMIT(v2 l6) +// -> compact by adding BIG_STABLE(v2 l2) +// 2) BIG_STABLE(v1 l1) DELETE(l2) BIG_UNSTABLE(v1 l3) ROLLBACK(v0 l4) +// -> compact by adding DELETE(l4) +// 3) BIG_STABLE(v1 l1) SMALL(v2 l2) SMALL(v3 l3) ROLLBACK(v2 l4) SMALL(v3 l5) COMMIT(v3 l6) +// -> compact by adding BIG_STABLE(v3 l6) and skip l3 +// 4) BIG_STABLE(v1 l1) SMALL_STABLE(v2 l2) BIG_UNSTABLE(v3 l3) +// -> skip compaction of l2 into l1 if not under pressure heap_compact_t blockstore_heap_t::iterate_compaction(heap_entry_t *obj, uint64_t fsynced_lsn, bool under_pressure, std::function small_wr_cb) { heap_compact_t res = {}; uint64_t commit_version = 0, rollback_version = UINT64_MAX; bool has_small = false; + res.do_delete = true; for (heap_entry_t *wr = obj; wr; wr = prev(wr)) { - // 1) 1 2 3 ROLLBACK(2) COMMIT(3) -> impossible - // 2) 1 2 3 4 ROLLBACK(3) COMMIT(2) -> OK - // 3) 1 2 3 ROLLBACK(2) 3 COMMIT(3) -> first 3 shouldn't be treated as stable - // 4) 1 2 3 COMMIT(3) ROLLBACK(2) -> impossible - // I.e. a rollback always has version >= previous commit - // 5) 1 2 3 4 5 ROLLBACK(4) 5 ROLLBACK(3) if (wr->type() == BS_HEAP_ROLLBACK) { if (wr->lsn <= fsynced_lsn && !res.compact_lsn) @@ -1596,54 +1500,57 @@ heap_compact_t blockstore_heap_t::iterate_compaction(heap_entry_t *obj, uint64_t res.compact_lsn = wr->lsn; res.compact_version = wr->version; } + res.do_delete = false; commit_version = wr->version; continue; } bool rolled_back = (wr->version > rollback_version); - bool stable = !rolled_back && ((wr->entry_type & BS_HEAP_STABLE) || (wr->version <= commit_version)); - if (!stable || wr->lsn > fsynced_lsn) + if (rolled_back) { - // Skip unstable or non-fsynced writes + continue; + } + bool stable = (wr->entry_type & BS_HEAP_STABLE); + bool committed = (wr->version <= commit_version); + if (!stable && !committed || wr->lsn > fsynced_lsn) + { + // Unstable and non-fsynced writes can't be compacted yet + res.do_delete = false; + res.compact_lsn = 0; + res.compact_version = 0; if (!under_pressure && (wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_DELETE)) { // We may postpone compaction if we have an unstable overwrite when not under pressure - res.compact_lsn = 0; - res.compact_version = 0; return res; } continue; } - if (wr->type() == BS_HEAP_BIG_WRITE) + if (wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_BIG_INTENT) { - // Stable big_write is here - res.clean_loc = wr->big_location(this); - res.clean_version = wr->version; - res.clean_lsn = wr->lsn; + // Big_write to merge small_writes into is here + if (!stable && !res.compact_lsn) + { + res.compact_lsn = wr->lsn; + res.compact_version = wr->version; + } + res.clean_wr = wr; + res.do_delete = false; return res; } if (wr->type() == BS_HEAP_DELETE) { // Object is deleted - assert(!has_small); - if (wr->entry_type & BS_HEAP_STABLE) - { - // Already have the stable bit, no need to generate a compaction entry - res.compact_lsn = 0; - res.compact_version = 0; - } + assert(!has_small && stable); // unstable deletes are not supported return res; } - // We finally have something compactable + assert(wr->type() == BS_HEAP_SMALL_WRITE || wr->type() == BS_HEAP_INTENT_WRITE); if (!res.compact_lsn) { res.compact_lsn = wr->lsn; res.compact_version = wr->version; } - if (wr->type() == BS_HEAP_SMALL_WRITE || wr->type() == BS_HEAP_INTENT_WRITE) - { - has_small = true; - small_wr_cb(wr); - } + res.do_delete = false; + has_small = true; + small_wr_cb(wr); } return res; } diff --git a/src/blockstore/blockstore_heap.h b/src/blockstore/blockstore_heap.h index 729ef7f7..e4faec88 100644 --- a/src/blockstore/blockstore_heap.h +++ b/src/blockstore/blockstore_heap.h @@ -132,7 +132,8 @@ struct heap_inflight_lsn_t struct heap_compact_t { uint64_t compact_lsn, compact_version; - uint64_t clean_lsn, clean_version, clean_loc; + heap_entry_t *clean_wr; + bool do_delete; }; struct heap_idx_t @@ -249,9 +250,10 @@ public: int add_big_intent(object_id oid, heap_entry_t *old_head, uint64_t version, uint32_t offset, uint32_t len, uint8_t *bitmap, uint8_t *data, uint8_t *checksums, uint32_t *modified_block); // adds a compacted up to entry to an object - int add_compact(heap_entry_t *obj, uint64_t to_lsn, uint32_t *modified_block, uint8_t *new_csums); - // "punch holes" in a big_entry and make a duplicate big_entry - int add_punch_holes(heap_entry_t *obj, uint64_t to_lsn, uint64_t version, uint8_t *new_bitmap, uint8_t *new_csums, uint32_t *modified_block); + int add_compact(heap_entry_t *obj, uint64_t compact_version, uint64_t compact_lsn, uint64_t compact_location, + bool do_delete, uint32_t *modified_block, uint8_t *new_int_bitmap, uint8_t *new_ext_bitmap, uint8_t *new_csums); + // "punch holes" in a big_entry + int punch_holes(heap_entry_t *wr, uint8_t *new_bitmap, uint8_t *new_csums, uint32_t *modified_block); // stabilize an unstable object version // return 0 if OK, ENOENT if not exists int add_commit(heap_entry_t *obj, uint64_t version, uint32_t *modified_block); diff --git a/src/blockstore/blockstore_read.cpp b/src/blockstore/blockstore_read.cpp index 0126f77c..48d14f18 100644 --- a/src/blockstore/blockstore_read.cpp +++ b/src/blockstore/blockstore_read.cpp @@ -192,7 +192,7 @@ uint32_t blockstore_impl_t::prepare_read_simple(std::vector & rea .disk_offset = start, .disk_len = end-start, .buf = buffer_area + wr->small().location + start - wr->small().offset, - .wr_lsn = wr->lsn, + .wr = wr, }); } else if (dsk.csum_block_size <= dsk.bitmap_granularity) @@ -255,7 +255,6 @@ uint32_t blockstore_impl_t::prepare_read_simple(std::vector & rea void blockstore_impl_t::prepare_disk_read(std::vector & read_vec, int pos, heap_entry_t *obj, heap_entry_t *wr, uint32_t blk_start, uint32_t blk_end, uint32_t start, uint32_t end, uint32_t copy_flags) { - // Only one INTENT_WRITE is allowed at a time uint64_t loc = 0; if (wr->type() == BS_HEAP_INTENT_WRITE) { @@ -283,7 +282,7 @@ void blockstore_impl_t::prepare_disk_read(std::vector & read_vec, .disk_loc = loc, .disk_offset = blk_start, .disk_len = blk_end - blk_start, - .wr_lsn = wr->lsn, + .wr = wr, }; if (blk_start != start || blk_end != end) { @@ -291,7 +290,7 @@ void blockstore_impl_t::prepare_disk_read(std::vector & read_vec, vec.copy_flags |= COPY_BUF_PADDED; if (pos > 0 && read_vec.size() >= pos && read_vec[pos-1].copy_flags == vec.copy_flags && - read_vec[pos-1].wr_lsn == vec.wr_lsn && + read_vec[pos-1].wr == vec.wr && read_vec[pos-1].disk_offset <= vec.disk_offset && read_vec[pos-1].disk_offset+read_vec[pos-1].disk_len >= blk_end) { @@ -383,7 +382,6 @@ void blockstore_impl_t::handle_read_event(ring_data_t *data, blockstore_op_t *op bool blockstore_impl_t::verify_read_checksums(blockstore_op_t *op) { - heap_entry_t *obj = heap->read_entry(op->oid); auto & rv = PRIV(op)->read_vec; for (auto & vec: rv) { @@ -393,16 +391,12 @@ bool blockstore_impl_t::verify_read_checksums(blockstore_op_t *op) memcpy(op->buf + vec.offset - op->offset, vec.buf + vec.offset - vec.disk_offset, vec.len); if (vec.copy_flags & (COPY_BUF_COALESCED|COPY_BUF_SKIP_CSUM)) continue; - heap_entry_t *wr = obj; - while (wr && wr->lsn != vec.wr_lsn) - wr = heap->prev(wr); - assert(wr); uint8_t *buf = vec.buf ? vec.buf : (op->buf + vec.offset - op->offset); - uint32_t *csums = (uint32_t*)(wr->get_checksums(heap) + uint32_t *csums = (uint32_t*)(vec.wr->get_checksums(heap) + (vec.disk_offset/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF) - - ((wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_BIG_INTENT) - ? 0 : (wr->small().offset/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF))); - if (!heap->calc_block_checksums(csums, buf, wr->get_int_bitmap(heap), + - ((vec.wr->type() == BS_HEAP_BIG_WRITE || vec.wr->type() == BS_HEAP_BIG_INTENT) + ? 0 : (vec.wr->small().offset/dsk.csum_block_size)*(dsk.data_csum_type & 0xFF))); + if (!heap->calc_block_checksums(csums, buf, vec.wr->get_int_bitmap(heap), vec.disk_offset, vec.disk_offset+vec.disk_len, false, [&](uint32_t mismatch_pos, uint32_t expected_csum, uint32_t real_csum) { printf( diff --git a/src/test/test_heap.cpp b/src/test/test_heap.cpp index a9c59425..33bd2c94 100644 --- a/src/test/test_heap.cpp +++ b/src/test/test_heap.cpp @@ -328,12 +328,16 @@ void test_defrag_block() // The next write should be rejected because allowing it would block compaction assert(_test_do_big_write(heap, dsk, 1, (nwr+1)*0x20000, 1, (nwr+1)*0x20000, true, 0, 0, buffer_area.data()) == ENOSPC); // Compact all small writes + uint8_t bitmap[dsk.clean_entry_bitmap_size]; + memset(bitmap, 0xFF, dsk.clean_entry_bitmap_size); uint32_t mblock = 999999; for (uint32_t i = 0; i < nwr; i++) { auto obj = heap.read_entry((object_id){ .inode = INODE_WITH_POOL(1, 1), .stripe = i*0x20000 }); assert(obj); - int res = heap.add_compact(obj, obj->lsn, &mblock, NULL); + assert(heap.prev(obj)->entry_type == (BS_HEAP_BIG_WRITE|BS_HEAP_STABLE)); + int res = heap.add_compact(obj, obj->version, obj->lsn, heap.prev(obj)->big_location(&heap), + false, &mblock, bitmap, bitmap, NULL); assert(res == 0); heap.start_block_write(mblock); heap.complete_block_write(mblock); @@ -414,8 +418,6 @@ void test_compact(bool csum, bool stable) int small_writes = 0; heap_entry_t *small_wr = NULL; // FIXME: Check more iterate_compaction cases, also check more compact_object cases - // At least: - // - BIG_STABLE[v=1 l=1] SMALL_U[v=2 l=2] SMALL_U[v=3 l=3] ROLLBACK[v=1 l=4] -> BIG_STABLE[l=4] auto compact_info = heap.iterate_compaction(obj, heap.get_fsynced_lsn(), false, [&](heap_entry_t *wr) { small_wr = wr; @@ -423,14 +425,20 @@ void test_compact(bool csum, bool stable) }); assert(compact_info.compact_lsn == (stable ? 2 : 4)); assert(compact_info.compact_version == 3); - assert(compact_info.clean_lsn == 1); - assert(compact_info.clean_version == 1); - assert(compact_info.clean_loc == 0x20000); + assert(compact_info.clean_wr->lsn == 1); assert(small_writes == 1); assert(small_wr->lsn == 2); - res = heap.add_compact(obj, 4 /*max_lsn*/, &mblock, NULL); - assert(res == 0); + bitmap_set(ref_int_bitmap, 8192, 4096, 4096); + { + uint32_t csums[dsk.data_block_size/(dsk.csum_block_size ? dsk.csum_block_size : 4096)] = {}; + csums[0] = crc32c(0, buffer_area.data(), 4096); + csums[2] = crc32c(0, buffer_area.data()+8192, 4096); + res = heap.add_compact(obj, compact_info.compact_version, compact_info.compact_lsn, + compact_info.clean_wr->big_location(&heap), compact_info.do_delete, + &mblock, ref_int_bitmap, ref_int_bitmap, (uint8_t*)csums); + assert(res == 0); + } assert(mblock == 0); heap.start_block_write(mblock); heap.complete_block_write(mblock); @@ -444,7 +452,6 @@ void test_compact(bool csum, bool stable) assert(obj); assert(count_writes(heap, obj) == (stable ? 3 : 4)); assert(obj->version == 3); - bitmap_set(ref_int_bitmap, 8192, 4096, 4096); assert(!memcmp(obj->get_int_bitmap(&heap), ref_int_bitmap, dsk.clean_entry_bitmap_size)); if (csum) { @@ -491,7 +498,7 @@ void test_modify_bitmap() new_csums[0] = crc32c(0, buffer_area.data(), 4096); uint32_t mblock = 999999; - int res = heap.add_punch_holes(obj, 999, 1, new_bmp, new_csums, &mblock); + int res = heap.punch_holes(heap.prev(obj), new_bmp, new_csums, &mblock); assert(res == 0); assert(mblock == 0); heap.start_block_write(mblock); @@ -499,7 +506,7 @@ void test_modify_bitmap() obj = heap.read_entry(oid); assert(obj); - assert(count_writes(heap, obj) == 2); // duplicate big_write should be inserted after previous big_write, but before small_writes + assert(count_writes(heap, obj) == 2); assert(memcmp(heap.prev(obj)->get_int_bitmap(&heap), new_bmp, dsk.clean_entry_bitmap_size) == 0); assert(memcmp(heap.prev(obj)->get_checksums(&heap), new_csums, dsk.data_block_size/32768*4) == 0); @@ -726,8 +733,11 @@ void test_full_overwrite(bool stable) assert(!heap.is_buffer_area_free(16384, 4096)); assert(!heap.is_buffer_area_free(20480, 4096)); + uint8_t bitmap[dsk.clean_entry_bitmap_size]; + memset(bitmap, 0xFF, dsk.clean_entry_bitmap_size); uint32_t mblock = 999999; - res = heap.add_compact(obj, 999, &mblock, NULL); + res = heap.add_compact(obj, obj->version, obj->lsn, wr->big_location(&heap), + false, &mblock, bitmap, bitmap, NULL); assert(res == 0); assert(mblock == 0); heap.start_block_write(mblock); @@ -921,6 +931,7 @@ void test_rollback() assert(!(wr->entry_type & BS_HEAP_STABLE)); wr = heap.prev(wr); assert(wr->version == 2); + assert(wr->lsn == 2); assert(wr->entry_type == BS_HEAP_SMALL_WRITE|BS_HEAP_STABLE); assert(wr->small().location == 16384); assert(wr->small().len == 4096); @@ -930,7 +941,10 @@ void test_rollback() assert(wr->big_location(&heap) == 0x20000); // compact without rollback (can we do it at all?) - res = heap.add_compact(obj, 4, &mblock, NULL); + uint8_t bitmap[dsk.clean_entry_bitmap_size]; + memset(bitmap, 0xFF, dsk.clean_entry_bitmap_size); + res = heap.add_compact(obj, 2, 2, wr->big_location(&heap), + false, &mblock, bitmap, bitmap, NULL); assert(res == 0); assert(mblock == 0); heap.start_block_write(mblock); @@ -978,7 +992,7 @@ void test_rollback() // But the data is still in place, removed only on compaction assert(heap.is_data_used(0x20000)); - res = heap.add_compact(obj, obj->lsn, &mblock, NULL); + res = heap.add_compact(obj, 0, 2, 0, true, &mblock, NULL, NULL, NULL); assert(res == 0); assert(mblock == 0); heap.start_block_write(mblock);