diff --git a/src/blockstore/blockstore_impl.cpp b/src/blockstore/blockstore_impl.cpp index 5861e09d..e2293671 100644 --- a/src/blockstore/blockstore_impl.cpp +++ b/src/blockstore/blockstore_impl.cpp @@ -137,6 +137,7 @@ void blockstore_impl_t::loop() has_unfinished_writes = has_unfinished_writes || op->opcode == BS_OP_WRITE || op->opcode == BS_OP_WRITE_STABLE || op->opcode == BS_OP_DELETE || op->opcode == BS_OP_STABLE || op->opcode == BS_OP_ROLLBACK; + has_unfinished_sync = has_unfinished_sync || op->opcode == BS_OP_SYNC; continue; } } diff --git a/src/blockstore/blockstore_sync.cpp b/src/blockstore/blockstore_sync.cpp index d34398d3..4f546d95 100644 --- a/src/blockstore/blockstore_sync.cpp +++ b/src/blockstore/blockstore_sync.cpp @@ -107,6 +107,16 @@ int blockstore_impl_t::do_sync(blockstore_op_t *op, int base_state) } if (!has_unsynced()) { + if ((!dsk.disable_meta_fsync || !dsk.disable_journal_fsync) && + heap->get_fsynced_lsn() < heap->get_completed_lsn()) + { + // Counters are reset when an fsync is submitted: completed writes may still + // be waiting for an fsync submitted by another operation + PRIV(op)->wait_for = WAIT_META_FSYNC; + PRIV(op)->wait_detail = heap->get_completed_lsn(); + flusher->request_fsync(); + return 0; + } // We can return immediately because sync only syncs previous writes unsynced_data_write_count = unsynced_buffer_write_count = unsynced_meta_write_count = 0; return 2; diff --git a/src/test/test_blockstore.cpp b/src/test/test_blockstore.cpp index 2730b5ab..e6537b05 100644 --- a/src/test/test_blockstore.cpp +++ b/src/test/test_blockstore.cpp @@ -2168,6 +2168,102 @@ static void test_big_write_after_redirect_intent() free(op2.buf); } +// A stabilize or rollback whose metadata write is covered by an fsync that another +// operation has submitted must wait for that fsync instead of completing right away +static void test_rollback_waits_for_inflight_fsync() +{ + printf("\n-- test_rollback_waits_for_inflight_fsync\n"); + + bs_test_t test; + test.default_cfg(); + test.config["disable_data_fsync"] = "0"; + test.config["immediate_commit"] = "none"; + test.init(); + + blockstore_op_t op; + op.opcode = BS_OP_WRITE; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = 1; + op.offset = 0; + op.len = 4096; + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + memset(op.buf, 0xaa, op.len); + test.exec_op(&op); + assert(op.retval == op.len); + op.oid = { .inode = 2, .stripe = 0 }; + test.exec_op(&op); + assert(op.retval == op.len); + blockstore_op_t sync_op; + sync_op.opcode = BS_OP_SYNC; + test.exec_op(&sync_op); + assert(sync_op.retval == 0); + + // Hold fsyncs + std::vector fsyncs; + test.sqe_handler = [&](io_uring_sqe *sqe) + { + if (sqe->opcode == IORING_OP_FSYNC) + { + fsyncs.push_back(*sqe); + return true; + } + return false; + }; + + // Both entries are in the same metadata block, so both operations resume after + // the same block write and the first one submits the fsync for both + obj_ver_id stab = { .oid = { .inode = 2, .stripe = 0 }, .version = 1 }; + obj_ver_id rb = { .oid = { .inode = 1, .stripe = 0 }, .version = 0 }; + bool stab_done = false, rb_done = false; + blockstore_op_t stab_op; + stab_op.opcode = BS_OP_STABLE; + stab_op.len = 1; + stab_op.buf = (uint8_t*)&stab; + stab_op.callback = [&](blockstore_op_t *op) { stab_done = true; }; + blockstore_op_t rb_op; + rb_op.opcode = BS_OP_ROLLBACK; + rb_op.len = 1; + rb_op.buf = (uint8_t*)&rb; + rb_op.callback = [&](blockstore_op_t *op) { rb_done = true; }; + test.bs->enqueue_op(&stab_op); + test.bs->enqueue_op(&rb_op); + for (int i = 0; i < 1000; i++) + test.ringloop->loop(); + assert(fsyncs.size() > 0); + assert(!stab_done); + assert(!rb_done); + + // Let fsyncs complete + test.sqe_handler = nullptr; + for (auto & sqe: fsyncs) + { + bool ok = test.data_disk->submit(&sqe); + assert(ok); + test.ringloop->mark_completed((ring_data_t*)sqe.user_data); + } + for (int i = 0; i < 1000 && (!stab_done || !rb_done); i++) + test.ringloop->loop(); + assert(stab_done && rb_done); + assert(stab_op.retval == 0 && rb_op.retval == 0); + + // The rollback survives a power outage + test.destroy_bs(); + test.data_disk->discard_buffers(true, 0); + test.init(); + blockstore_op_t rd; + rd.opcode = BS_OP_READ; + rd.oid = { .inode = 1, .stripe = 0 }; + rd.version = UINT64_MAX; + rd.offset = 0; + rd.len = 4096; + rd.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, rd.len); + test.exec_op(&rd); + assert(rd.retval == -ENOENT); + + free(op.buf); + free(rd.buf); +} + int main(int narg, char *args[]) { test_simple(); @@ -2198,6 +2294,7 @@ int main(int narg, char *args[]) test_zero_write_over_rollback_to_zero(); test_compact_intent_write_in_partial_csum_block(); test_no_intent_over_unwritten_compaction(); + test_rollback_waits_for_inflight_fsync(); test_big_write_after_redirect_intent(); return 0; }