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;
                 }
             }

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;

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<io_uring_sqe> 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;
 }