From 572b20fedce5d96bf006e336de3ef4b582af2a94 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sat, 16 May 2026 13:39:23 +0300 Subject: [PATCH] Fix mark_partial_write for replicas (TODO: try to write an OSD unit test for it) --- src/client/msgr_op.h | 1 + src/osd/osd.h | 3 +-- src/osd/osd_primary.cpp | 31 ++++++++++++++++---------- src/osd/osd_primary_subops.cpp | 40 +++++++++++++++++++--------------- src/osd/osd_primary_write.cpp | 7 +++++- 5 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/client/msgr_op.h b/src/client/msgr_op.h index 020b9530..ce8c2869 100644 --- a/src/client/msgr_op.h +++ b/src/client/msgr_op.h @@ -157,6 +157,7 @@ struct __attribute__((visibility("default"))) osd_op_t timespec tv_begin = { 0 }, tv_end = { 0 }; uint64_t op_type = OSD_OP_IN; uint64_t client_id = 0; + osd_num_t osd_num = 0; osd_any_op_t req; osd_any_reply_t reply; blockstore_op_t *bs_op = NULL; diff --git a/src/osd/osd.h b/src/osd/osd.h index 18ac0936..2c5430ee 100644 --- a/src/osd/osd.h +++ b/src/osd/osd.h @@ -352,8 +352,7 @@ class osd_t std::function calc_set); pg_osd_set_state_t *mark_object_corrupted(pg_t & pg, object_id oid, pg_osd_set_state_t *prev_object_state, osd_rmw_stripe_t *stripes, bool ref); - pg_osd_set_state_t *mark_partial_write(pg_t & pg, object_id oid, pg_osd_set_state_t *prev_object_state, - osd_rmw_stripe_t *stripes, bool ref); + pg_osd_set_state_t *mark_partial_write(pg_t & pg, osd_op_t *cur_op); void deref_object_state(pg_t & pg, pg_osd_set_state_t **object_state, bool deref); bool remember_unstable_write(osd_op_t *cur_op, pg_t & pg, pg_osd_set_t & loc_set, int base_state); void handle_primary_subop(osd_op_t *subop, osd_op_t *cur_op); diff --git a/src/osd/osd_primary.cpp b/src/osd/osd_primary.cpp index b64391b1..f4373a13 100644 --- a/src/osd/osd_primary.cpp +++ b/src/osd/osd_primary.cpp @@ -435,24 +435,33 @@ pg_osd_set_state_t *osd_t::mark_object_corrupted(pg_t & pg, object_id oid, } // Mark the object as partially updated (probably due to a ENOSPC) -pg_osd_set_state_t *osd_t::mark_partial_write(pg_t & pg, object_id oid, pg_osd_set_state_t *prev_object_state, - osd_rmw_stripe_t *stripes, bool ref) +pg_osd_set_state_t *osd_t::mark_partial_write(pg_t & pg, osd_op_t *cur_op) { - return mark_object(pg, oid, prev_object_state, ref, [stripes](pg_osd_set_t & new_set) + osd_primary_op_data_t *op_data = cur_op->op_data; + return mark_object(pg, op_data->oid, op_data->object_state, true, [&](pg_osd_set_t & new_set) { // Mark object chunk(s) as outdated int changes = 0; - for (auto chunk_it = new_set.begin(); chunk_it != new_set.end(); ) + for (auto & chunk: new_set) { - auto & chunk = *chunk_it; - if (stripes[chunk.role].osd_num == chunk.osd_num && - stripes[chunk.role].read_error && - chunk.loc_bad != LOC_OUTDATED) + if (chunk.loc_bad != LOC_OUTDATED) { - changes++; - chunk.loc_bad = LOC_OUTDATED; + bool success = false; + for (int i = 0; i < op_data->n_subops; i++) + { + if (op_data->subops[i].osd_num == chunk.osd_num && + op_data->subops[i].reply.hdr.retval == op_data->subops[i].req.sec_rw.len) + { + success = true; + break; + } + } + if (!success) + { + changes++; + chunk.loc_bad = LOC_OUTDATED; + } } - chunk_it++; } return changes; }); diff --git a/src/osd/osd_primary_subops.cpp b/src/osd/osd_primary_subops.cpp index e401c965..88799c5e 100644 --- a/src/osd/osd_primary_subops.cpp +++ b/src/osd/osd_primary_subops.cpp @@ -276,6 +276,7 @@ void osd_t::submit_primary_subop(osd_op_t *cur_op, osd_op_t *subop, bool osd_t::submit_to_osd(osd_op_t *subop, osd_num_t osd_num) { + subop->osd_num = osd_num; auto peer_it = msgr.osd_peers.find(osd_num); if (peer_it != msgr.osd_peers.end()) { @@ -489,8 +490,11 @@ void osd_t::handle_primary_subop(osd_op_t *subop, osd_op_t *cur_op) } if ((op_data->errors + op_data->done) >= op_data->n_subops) { - delete[] op_data->subops; - op_data->subops = NULL; + if (!op_data->errors || !op_data->done || opcode != OSD_OP_SEC_WRITE && opcode != OSD_OP_SEC_WRITE_STABLE) + { + delete[] op_data->subops; + op_data->subops = NULL; + } op_data->st++; if (cur_op->req.hdr.opcode == OSD_OP_READ) { @@ -641,45 +645,47 @@ void osd_t::submit_primary_sync_subops(osd_op_t *cur_op) osd_op_t *subops = new osd_op_t[n_osds]; op_data->subops = subops; robin_hood::unordered_flat_map::iterator peer_it; + int subop_idx = 0; for (int i = 0; i < n_osds; i++) { osd_num_t sync_osd = op_data->dirty_osds[i]; + osd_op_t *subop = &subops[subop_idx]; if (sync_osd == this->osd_num) { - clock_gettime(CLOCK_REALTIME, &subops[i].tv_begin); - subops[i].op_type = (uint64_t)cur_op; - subops[i].bs_op = new blockstore_op_t({ + clock_gettime(CLOCK_REALTIME, &subop->tv_begin); + subop->op_type = (uint64_t)cur_op; + subop->bs_op = new blockstore_op_t({ .opcode = BS_OP_SYNC, - .callback = [subop = &subops[i], this](blockstore_op_t *bs_subop) + .callback = [subop, this](blockstore_op_t *bs_subop) { handle_primary_bs_subop(subop); }, }); - bs->enqueue_op(subops[i].bs_op); + bs->enqueue_op(subop->bs_op); + subop_idx++; } else if ((peer_it = msgr.osd_peers.find(sync_osd)) != msgr.osd_peers.end()) { - subops[i].op_type = OSD_OP_OUT; - subops[i].client_id = peer_it->second->client_id; - subops[i].req = (osd_any_op_t){ .sec_sync = { + subop->op_type = OSD_OP_OUT; + subop->osd_num = sync_osd; + subop->client_id = peer_it->second->client_id; + subop->req = (osd_any_op_t){ .sec_sync = { .header = { .magic = SECONDARY_OSD_OP_MAGIC, .opcode = OSD_OP_SEC_SYNC, }, .flags = cur_op->client_id == SELF_CLIENT && cur_op->req.hdr.opcode != OSD_OP_SCRUB ? OSD_OP_RECOVERY_RELATED : 0, } }; - subops[i].callback = [cur_op, this](osd_op_t *subop) + subop->callback = [cur_op, this](osd_op_t *subop) { handle_primary_subop(subop, cur_op); }; - msgr.outbox_push(&subops[i]); - } - else - { - op_data->n_subops--; + msgr.outbox_push(subop); + subop_idx++; } } - if (op_data->n_subops <= 0) + op_data->n_subops = subop_idx; + if (subop_idx <= 0) { delete[] op_data->subops; op_data->subops = NULL; diff --git a/src/osd/osd_primary_write.cpp b/src/osd/osd_primary_write.cpp index 68b8294c..d0fb0218 100644 --- a/src/osd/osd_primary_write.cpp +++ b/src/osd/osd_primary_write.cpp @@ -263,11 +263,16 @@ resume_12: else { pg.ver_override.erase(op_data->oid); - mark_partial_write(pg, op_data->oid, op_data->object_state, op_data->stripes, true); + mark_partial_write(pg, cur_op); pg_cancel_write_queue(pg, cur_op, op_data->oid, op_data->errcode); return; } } + if (op_data->subops) + { + delete[] op_data->subops; + op_data->subops = NULL; + } pg.ver_override.erase(op_data->oid); deref_object_state(pg, &op_data->object_state, true); pg_cancel_write_queue(pg, cur_op, op_data->oid, op_data->errcode);