From eaff4509cacbaae09144d545d6943d2f6084d502 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Thu, 6 Nov 2025 01:32:23 +0300 Subject: [PATCH] Complete small/intent writes explicitly, not implicitly on metadata block write --- src/blockstore/blockstore_heap.cpp | 69 +++++++++++++++++++---------- src/blockstore/blockstore_heap.h | 7 +-- src/blockstore/blockstore_impl.h | 5 ++- src/blockstore/blockstore_read.cpp | 8 ++-- src/blockstore/blockstore_write.cpp | 22 ++++++--- src/test/test_heap.cpp | 13 ++++-- 6 files changed, 83 insertions(+), 41 deletions(-) diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index 5dea98c3..cf51290e 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -23,6 +23,7 @@ #define HEAP_INFLIGHT_COMPACTABLE 2 #define HEAP_INFLIGHT_COMPACTED 4 #define HEAP_INFLIGHT_GC 8 +#define HEAP_INFLIGHT_EXPLICIT 16 static inline heap_list_item_t *list_item(heap_entry_t *wr) { @@ -697,7 +698,7 @@ bool blockstore_heap_t::recheck_small_writes(std::functiontype() == BS_HEAP_BIG_INTENT) { auto & bi = wr->big_intent(); - loc = bi.block_num * dsk->data_block_size + bi.offset; + loc = (uint64_t)bi.block_num * dsk->data_block_size + bi.offset; len = bi.len; from_data = true; } @@ -1147,7 +1148,7 @@ void blockstore_heap_t::insert_list_item(heap_list_item_t *li) } int blockstore_heap_t::add_entry(uint32_t wr_size, uint32_t *modified_block, - bool allow_last_free, std::function fill_entry) + bool allow_last_free, bool explicit_complete, std::function fill_entry) { uint32_t block_num; int res = allocate_entry(wr_size, &block_num, allow_last_free); @@ -1172,6 +1173,7 @@ int blockstore_heap_t::add_entry(uint32_t wr_size, uint32_t *modified_block, inf.mod_lsn_to = next_lsn; // Remember the object as dirty and remove older entries when this block is written and fsynced push_inflight_lsn(next_lsn, new_wr, + (explicit_complete ? HEAP_INFLIGHT_EXPLICIT : 0) | (new_wr->is_overwrite() ? HEAP_INFLIGHT_COMPACTED : 0) | (new_wr->is_compactable() ? HEAP_INFLIGHT_COMPACTABLE : 0)); insert_list_item(li); @@ -1183,17 +1185,19 @@ int blockstore_heap_t::add_entry(uint32_t wr_size, uint32_t *modified_block, // 1st step: post a write -int blockstore_heap_t::add_small_write(object_id oid, heap_entry_t *old_head, uint16_t type, uint64_t version, +int blockstore_heap_t::add_small_write(object_id oid, heap_entry_t **obj_ptr, uint16_t type, uint64_t version, uint32_t offset, uint32_t len, uint64_t location, uint8_t *bitmap, uint8_t *data, uint32_t *modified_block) { - if (!old_head || old_head->type() == BS_HEAP_DELETE || old_head->version > version || + auto obj = *obj_ptr; + if (!obj || obj->type() == BS_HEAP_DELETE || obj->version > version || type != (BS_HEAP_SMALL_WRITE|BS_HEAP_STABLE) && type != BS_HEAP_SMALL_WRITE && type != (BS_HEAP_INTENT_WRITE|BS_HEAP_STABLE) || - (type & BS_HEAP_STABLE) && !(old_head->entry_type & BS_HEAP_STABLE)) + (type & BS_HEAP_STABLE) && !(obj->entry_type & BS_HEAP_STABLE)) { return EINVAL; } uint32_t wr_size = get_small_entry_size(offset, len); - return add_entry(wr_size, modified_block, false, [&](heap_entry_t *wr) + // Small writes are written in parallel with buffered data so they require explicit_complete + return add_entry(wr_size, modified_block, false, true, [&](heap_entry_t *wr) { wr->entry_type = type; wr->inode = oid.inode; @@ -1204,11 +1208,12 @@ int blockstore_heap_t::add_small_write(object_id oid, heap_entry_t *old_head, ui wr->small().location = location; if (bitmap) memcpy(wr->get_ext_bitmap(this), bitmap, dsk->clean_entry_bitmap_size); - else if (old_head) - memcpy(wr->get_ext_bitmap(this), old_head->get_ext_bitmap(this), dsk->clean_entry_bitmap_size); + else if (obj) + memcpy(wr->get_ext_bitmap(this), obj->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); + *obj_ptr = wr; }); } @@ -1220,7 +1225,8 @@ int blockstore_heap_t::add_big_write(object_id oid, heap_entry_t *old_head, bool return EINVAL; } uint32_t wr_size = get_big_entry_size(); - return add_entry(wr_size, modified_block, false, [&](heap_entry_t *wr) + // Big writes are written after writing data so they don't require explicit_complete + return add_entry(wr_size, modified_block, false, false, [&](heap_entry_t *wr) { wr->entry_type = BS_HEAP_BIG_WRITE | (stable ? BS_HEAP_STABLE : 0); wr->inode = oid.inode; @@ -1241,18 +1247,20 @@ int blockstore_heap_t::add_big_write(object_id oid, heap_entry_t *old_head, bool }); } -int blockstore_heap_t::add_big_intent(object_id oid, heap_entry_t *old_head, uint64_t version, +int blockstore_heap_t::add_big_intent(object_id oid, heap_entry_t **obj_ptr, uint64_t version, uint32_t offset, uint32_t len, uint8_t *bitmap, uint8_t *data, uint8_t *checksums, uint32_t *modified_block) { - if (!old_head || - old_head->entry_type != (BS_HEAP_BIG_INTENT|BS_HEAP_STABLE) && - old_head->entry_type != (BS_HEAP_BIG_WRITE|BS_HEAP_STABLE) || + auto obj = *obj_ptr; + if (!obj || + obj->entry_type != (BS_HEAP_BIG_INTENT|BS_HEAP_STABLE) && + obj->entry_type != (BS_HEAP_BIG_WRITE|BS_HEAP_STABLE) || dsk->csum_block_size > dsk->bitmap_granularity && !checksums) { return EINVAL; } uint32_t wr_size = get_big_intent_entry_size(); - return add_entry(wr_size, modified_block, false, [&](heap_entry_t *wr) + // Big intents are written before writing data so they require explicit_complete + return add_entry(wr_size, modified_block, false, true, [&](heap_entry_t *wr) { wr->entry_type = BS_HEAP_BIG_INTENT | BS_HEAP_STABLE; wr->inode = oid.inode; @@ -1261,14 +1269,14 @@ int blockstore_heap_t::add_big_intent(object_id oid, heap_entry_t *old_head, uin auto & bi = wr->big_intent(); bi.offset = offset; bi.len = len; - bi.block_num = (old_head->type() == BS_HEAP_BIG_INTENT - ? old_head->big_intent().block_num - : old_head->big().block_num); + bi.block_num = (obj->type() == BS_HEAP_BIG_INTENT + ? obj->big_intent().block_num + : obj->big().block_num); if (bitmap) memcpy(wr->get_ext_bitmap(this), bitmap, dsk->clean_entry_bitmap_size); else - memcpy(wr->get_ext_bitmap(this), old_head->get_ext_bitmap(this), dsk->clean_entry_bitmap_size); - memcpy(wr->get_int_bitmap(this), old_head->get_int_bitmap(this), dsk->clean_entry_bitmap_size); + memcpy(wr->get_ext_bitmap(this), obj->get_ext_bitmap(this), dsk->clean_entry_bitmap_size); + memcpy(wr->get_int_bitmap(this), obj->get_int_bitmap(this), dsk->clean_entry_bitmap_size); bitmap_set(wr->get_int_bitmap(this), offset, len, dsk->bitmap_granularity); if (dsk->data_csum_type) { @@ -1276,12 +1284,13 @@ int blockstore_heap_t::add_big_intent(object_id oid, heap_entry_t *old_head, uin memcpy(wr->get_checksums(this), checksums, dsk->clean_entry_bitmap_size); else { - memcpy(wr->get_checksums(this), old_head->get_checksums(this), dsk->clean_entry_bitmap_size); + memcpy(wr->get_checksums(this), obj->get_checksums(this), dsk->clean_entry_bitmap_size); calc_checksums(wr, (uint8_t*)data, true, offset, len); } } else calc_checksums(wr, (uint8_t*)data, true); + *obj_ptr = wr; }); } @@ -1290,7 +1299,7 @@ int blockstore_heap_t::add_compact(heap_entry_t *obj, uint64_t compact_version, { if (do_delete) { - return add_entry(get_simple_entry_size(), modified_block, false, [&](heap_entry_t *wr) + return add_entry(get_simple_entry_size(), modified_block, false, false, [&](heap_entry_t *wr) { wr->entry_type = BS_HEAP_DELETE|BS_HEAP_STABLE; wr->inode = obj->inode; @@ -1300,7 +1309,8 @@ int blockstore_heap_t::add_compact(heap_entry_t *obj, uint64_t compact_version, }); } uint32_t wr_size = get_big_entry_size(); - return add_entry(wr_size, modified_block, true, [&](heap_entry_t *new_wr) + // Compaction entry is added after copying data so it doesn't require explicit_complete + return add_entry(wr_size, modified_block, true, false, [&](heap_entry_t *new_wr) { new_wr->entry_type = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; new_wr->inode = obj->inode; @@ -1335,7 +1345,8 @@ int blockstore_heap_t::punch_holes(heap_entry_t *wr, uint8_t *new_bitmap, uint8_ int blockstore_heap_t::add_simple(heap_entry_t *obj, uint64_t version, uint32_t *modified_block, uint32_t entry_type) { uint32_t wr_size = get_simple_entry_size(); - return add_entry(wr_size, modified_block, false, [&](heap_entry_t *wr) + // Simple entries don't have data so they don't require explicit_complete + return add_entry(wr_size, modified_block, false, false, [&](heap_entry_t *wr) { wr->entry_type = entry_type; wr->inode = obj->inode; @@ -1518,12 +1529,22 @@ void blockstore_heap_t::complete_block_write(uint32_t block_num) for (uint64_t lsn = mod_lsn; lsn <= mod_lsn_to; lsn++, it++) { assert(!(it->flags & HEAP_INFLIGHT_DONE)); - it->flags |= HEAP_INFLIGHT_DONE; + if (!(it->flags & HEAP_INFLIGHT_EXPLICIT)) + it->flags |= HEAP_INFLIGHT_DONE; } mark_completed_lsns(mod_lsn); } } +void blockstore_heap_t::complete_lsn_write(uint64_t lsn) +{ + auto it = inflight_lsn.begin() + (lsn-first_inflight_lsn); + assert(!(it->flags & HEAP_INFLIGHT_DONE)); + assert(it->flags & HEAP_INFLIGHT_EXPLICIT); + it->flags |= HEAP_INFLIGHT_DONE; + mark_completed_lsns(lsn); +} + void blockstore_heap_t::mark_garbage_up_to(heap_entry_t *wr) { auto mvcc_it = object_mvcc.find((object_id){ .inode = wr->inode, .stripe = wr->stripe }); diff --git a/src/blockstore/blockstore_heap.h b/src/blockstore/blockstore_heap.h index 2a689fe9..3723aa13 100644 --- a/src/blockstore/blockstore_heap.h +++ b/src/blockstore/blockstore_heap.h @@ -213,7 +213,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); int add_entry(uint32_t wr_size, uint32_t *modified_block, bool allow_last_free, - std::function fill_entry); + 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); uint32_t meta_alloc_pos(const heap_block_info_t & inf); void modify_alloc(uint32_t block_num, std::function change_cb); @@ -260,13 +260,13 @@ public: bool set, std::function bad_block_cb); // adds a small_write or intent_write entry to an object // return 0 if OK, or maybe ENOSPC - int add_small_write(object_id oid, heap_entry_t *old_head, uint16_t type, uint64_t version, + int add_small_write(object_id oid, heap_entry_t **obj_ptr, uint16_t type, uint64_t version, uint32_t offset, uint32_t len, uint64_t location, uint8_t *bitmap, uint8_t *data, uint32_t *modified_block); // adds a big_write (overwrite) entry to an object int add_big_write(object_id oid, heap_entry_t *old_head, bool stable, uint64_t version, uint32_t offset, uint32_t len, uint64_t location, uint8_t *bitmap, uint8_t *data, uint32_t *modified_block); // adds a big_intent (atomic partial modification) entry to an object - int add_big_intent(object_id oid, heap_entry_t *old_head, uint64_t version, + int add_big_intent(object_id oid, heap_entry_t **obj_ptr, 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 compact_version, uint64_t compact_lsn, uint64_t compact_location, @@ -299,6 +299,7 @@ public: // inflight write tracking void start_block_write(uint32_t block_num); void complete_block_write(uint32_t block_num); + void complete_lsn_write(uint64_t lsn); uint64_t get_completed_lsn(); uint64_t get_fsynced_lsn(); void mark_lsn_fsynced(uint64_t lsn); diff --git a/src/blockstore/blockstore_impl.h b/src/blockstore/blockstore_impl.h index e67309c7..a8dd3be1 100644 --- a/src/blockstore/blockstore_impl.h +++ b/src/blockstore/blockstore_impl.h @@ -47,8 +47,11 @@ struct blockstore_op_private_t // Read std::vector read_vec; + // Read, write + uint64_t lsn; + // Write - uint64_t location; // or lsn for read + uint64_t location; uint32_t write_type; // Stabilize, rollback diff --git a/src/blockstore/blockstore_read.cpp b/src/blockstore/blockstore_read.cpp index 78c1d35f..b433c89e 100644 --- a/src/blockstore/blockstore_read.cpp +++ b/src/blockstore/blockstore_read.cpp @@ -17,7 +17,7 @@ int blockstore_impl_t::dequeue_read(blockstore_op_t *op) } uint32_t fulfilled = 0; PRIV(op)->pending_ops = 0; - PRIV(op)->location = 0; + PRIV(op)->lsn = 0; auto & rv = PRIV(op)->read_vec; uint64_t result_version = 0; bool found = false; @@ -26,7 +26,7 @@ int blockstore_impl_t::dequeue_read(blockstore_op_t *op) bool need_skip = dsk.csum_block_size > dsk.bitmap_granularity && !perfect_csum_update; if (need_skip) { - PRIV(op)->location = obj->lsn; + PRIV(op)->lsn = obj->lsn; blk_start = op->offset - op->offset%dsk.csum_block_size; blk_end = op->offset + op->len; if (blk_end % dsk.csum_block_size) @@ -395,10 +395,10 @@ 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) { bool skip_all = false; - if (PRIV(op)->location) + if (PRIV(op)->lsn) { heap_entry_t *obj = heap->read_entry(op->oid); - if (obj->lsn != PRIV(op)->location) // check top lsn + if (obj->lsn != PRIV(op)->lsn) // check top lsn { skip_all = true; } diff --git a/src/blockstore/blockstore_write.cpp b/src/blockstore/blockstore_write.cpp index 59e94eaa..d2791de7 100644 --- a/src/blockstore/blockstore_write.cpp +++ b/src/blockstore/blockstore_write.cpp @@ -211,8 +211,12 @@ enospc: // FIXME: Support RMW mode for csum_block_size > bitmap_granularity PRIV(op)->write_type = BS_HEAP_BIG_INTENT; PRIV(op)->location = obj->big_location(heap); - res = heap->add_big_intent(op->oid, obj, op->version, op->offset, op->len, op->bitmap, + res = heap->add_big_intent(op->oid, &obj, op->version, op->offset, op->len, op->bitmap, (uint8_t*)op->buf, NULL, &PRIV(op)->modified_block); + if (res == ENOSPC) + goto enospc; + assert(res == 0); + PRIV(op)->lsn = obj->lsn; } else { @@ -224,12 +228,13 @@ enospc: } assert(wr && (wr->type() == BS_HEAP_BIG_WRITE || wr->type() == BS_HEAP_BIG_INTENT)); PRIV(op)->location = wr->big_location(heap); - res = heap->add_small_write(op->oid, obj, (BS_HEAP_INTENT_WRITE | (op->opcode == BS_OP_WRITE_STABLE ? BS_HEAP_STABLE : 0)), + res = heap->add_small_write(op->oid, &obj, (BS_HEAP_INTENT_WRITE | (op->opcode == BS_OP_WRITE_STABLE ? BS_HEAP_STABLE : 0)), op->version, op->offset, op->len, 0, op->bitmap, (uint8_t*)op->buf, &PRIV(op)->modified_block); + if (res == ENOSPC) + goto enospc; + assert(res == 0); + PRIV(op)->lsn = obj->lsn; } - if (res == ENOSPC) - goto enospc; - assert(res == 0); prepare_meta_block_write(PRIV(op)->modified_block); intent_write_counter++; PRIV(op)->pending_ops++; @@ -251,11 +256,12 @@ enospc: } // There is sufficient space. Check SQE(s) BS_SUBMIT_CHECK_SQES(1 + (op->len > 0 ? 1 : 0)); - int res = heap->add_small_write(op->oid, obj, (BS_HEAP_SMALL_WRITE | (op->opcode == BS_OP_WRITE_STABLE ? BS_HEAP_STABLE : 0)), + int res = heap->add_small_write(op->oid, &obj, (BS_HEAP_SMALL_WRITE | (op->opcode == BS_OP_WRITE_STABLE ? BS_HEAP_STABLE : 0)), op->version, op->offset, op->len, loc, op->bitmap, (uint8_t*)op->buf, &PRIV(op)->modified_block); if (res == ENOSPC) goto enospc; assert(res == 0); + PRIV(op)->lsn = obj->lsn; if (op->len) heap->use_buffer_area(op->oid.inode, loc, op->len); prepare_meta_block_write(PRIV(op)->modified_block); @@ -418,6 +424,10 @@ resume_8: printf("Ack write %jx:%jx v%ju\n", op->oid.inode, op->oid.stripe, op->version); #endif op->retval = op->len; + if (PRIV(op)->write_type == BS_HEAP_BIG_INTENT || + PRIV(op)->write_type == BS_HEAP_INTENT_WRITE || + PRIV(op)->write_type == BS_HEAP_SMALL_WRITE) + heap->complete_lsn_write(PRIV(op)->lsn); if (PRIV(op)->write_type == BS_HEAP_BIG_WRITE || PRIV(op)->write_type == BS_HEAP_BIG_INTENT || PRIV(op)->write_type == BS_HEAP_INTENT_WRITE) diff --git a/src/test/test_heap.cpp b/src/test/test_heap.cpp index 55661ccf..ac2cf52e 100644 --- a/src/test/test_heap.cpp +++ b/src/test/test_heap.cpp @@ -73,10 +73,13 @@ void _test_big_write(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint64_t } int _test_do_small_write(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint64_t inode, uint64_t stripe, uint64_t version, - uint32_t offset, uint32_t len, uint64_t location, bool stable, uint8_t *data, bool is_intent = false, uint32_t *mblock = NULL) + uint32_t offset, uint32_t len, uint64_t location, bool stable, uint8_t *data, bool is_intent = false, uint32_t *mblock = NULL, heap_entry_t **obj = NULL) { object_id oid = { .inode = INODE_WITH_POOL(1, inode), .stripe = stripe }; - heap_entry_t *obj = heap.read_entry(oid); + heap_entry_t *local_obj; + if (!obj) + obj = &local_obj; + *obj = heap.read_entry(oid); uint16_t type = (is_intent ? BS_HEAP_INTENT_WRITE : BS_HEAP_SMALL_WRITE) | (stable ? BS_HEAP_STABLE : 0); uint8_t ext_bitmap[dsk.clean_entry_bitmap_size]; memset(ext_bitmap, 0xff, dsk.clean_entry_bitmap_size); @@ -90,13 +93,15 @@ void _test_small_write(blockstore_heap_t & heap, blockstore_disk_t & dsk, uint64 if (!is_intent) heap.use_buffer_area(INODE_WITH_POOL(1, inode), location, len); // blocks are allocated before write and outside the heap_t uint32_t mblock = 999999; - int res = _test_do_small_write(heap, dsk, inode, stripe, version, offset, len, location, stable, data, is_intent, &mblock); + heap_entry_t *obj = NULL; + int res = _test_do_small_write(heap, dsk, inode, stripe, version, offset, len, location, stable, data, is_intent, &mblock, &obj); assert(res == 0); if (!is_intent) assert(!heap.is_buffer_area_free(location, len)); assert(mblock == expected_mblock || expected_mblock == UINT32_MAX); heap.start_block_write(mblock); heap.complete_block_write(mblock); + heap.complete_lsn_write(obj->lsn); } void _test_init(blockstore_disk_t & dsk, bool csum, std::function &)> cfg_cb = NULL) @@ -1234,6 +1239,8 @@ void test_intent_write(bool csum) printf("OK test_intent_write %s\n", csum ? "csum" : "no_csum"); } +// 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[]) { test_mvcc(false);