From: Alex Ainscow Date: Tue, 9 Jun 2026 14:45:25 +0000 (+0100) Subject: osd/ECTransaction: fix truncate+write planning for EC shard sizes X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=51d8c5c489ba3e664209fb3316f8d6e03e257e28;p=ceph.git osd/ECTransaction: fix truncate+write planning for EC shard sizes There are multiple problems fixed here, which are caused by operations which perform a truncate-then-write in a single transaction. NOTE: The only known scenario for these operations are CLS-sparsify operations. I recommend backporting these changes to Tentacle, however, as they are regressions in the RADOS API which can lead to data corruption. PROBLEM: Cache not invalidated correctly: If projected_size >= orig_size, the invalidate_cache flag is not set in the plan This means there is potentially data in the RMW extent cache, which may cause data corruption in subsequent writes (although not the write being made). No actual use case for this operation is known, so it may not be as serious as it sounds. FIX: Invalidate cache on any truncate or "delete_first" PROBLEM: Parity writes permitted with truncates: Currently parity delta writes do not support operations which may have truncates. However, if the object ended up the same size as pre-truncate, a parity delta write may have been attemtped. This may lead to data corruption. FIX: Block parity writes in this case. NOTE: It is not worth the development effort to support PDW in this scenario, as performance benefit would be minimal overall. PROBLEM: RMW Reads can occur beyond first truncate point. Such reads reflect the pre-truncate data (which is incorrect) and be preserved, even if the invalidate cache flag is set (since the cache is invalidated before the reads). This can lead to similar corruption as the earlier "cache not invalidated" FIX: trim the read to the lower truncate size. PROBLEM: When performing a truncate, the parity shards may need to be updated to reflect truncates on other shards. These truncate writes in the plan were overriding, rather than adding to, other writes in the op. The result was a potentially truncated coding shard. This leads to assertions on reads. FIX: Replace = with insert() PROBLEM: Some shards not set to correct size on truncate-then-write If projected_size is smaller or equal to the original size, then the code which attempts to correctly size a shard will not run. However if an operation performs a truncate then a partial write and does not write to a particular shard AND the shard ends up smaller, then this can lead to an incorrectly sized shard. This leads to assertion on reads. FIX: Execute the shard-resize code on all truncates. AI assistance was mainly used to write unit test. However, I cannot rule out a contribution to the simple fixes found in this commit, so out of caution, I place the Assisted-by tag. Fixes: https://tracker.ceph.com/issues/77276 Signed-off-by: Alex Ainscow Assisted-by: IBM-Bob:ClaudeSonnet/GPT --- diff --git a/src/osd/ECTransaction.cc b/src/osd/ECTransaction.cc index 8368a219040..ed6551b711a 100644 --- a/src/osd/ECTransaction.cc +++ b/src/osd/ECTransaction.cc @@ -149,7 +149,7 @@ ECTransaction::WritePlanObj::WritePlanObj( * 2. ALL delete operations (do NOT use is_delete() here!!!) * 3. Truncates that reduce size. */ - invalidates_cache = op.has_source(&source) || op.delete_first || projected_size < orig_size; + invalidates_cache = op.has_source(&source) || op.delete_first || (op.truncate && op.truncate->first < orig_size); op.buffer_updates.to_interval_set(unaligned_ro_writes); @@ -215,7 +215,7 @@ ECTransaction::WritePlanObj::WritePlanObj( /* Here we decide if we want to do a conventional write or a parity delta write. */ if (sinfo.supports_parity_delta_writes() && !object_in_cache && - orig_size == projected_size && !reads.empty()) { + orig_size == projected_size && !reads.empty() && !op.truncate) { shard_id_set read_shards = reads.get_shard_id_set(); shard_id_set pdw_read_shards = pdw_reads.get_shard_id_set(); @@ -267,6 +267,15 @@ ECTransaction::WritePlanObj::WritePlanObj( * read the existing data on the partial stripe. */ if (op.truncate && op.truncate->first < orig_size) { + if (to_read) { + ECUtil::shard_extent_set_t truncate_mask(sinfo.get_k_plus_m()); + sinfo.ro_range_to_shard_extent_set(0, op.truncate->first, truncate_mask); + + to_read->intersection_of(truncate_mask); + if (to_read->empty()) { + to_read = std::nullopt; + } + } ECUtil::shard_extent_set_t truncate_read(sinfo.get_k_plus_m()); uint64_t prev_stripe = sinfo.ro_offset_to_prev_stripe_ro_offset(op.truncate->first); uint64_t next_align = ECUtil::align_next(op.truncate->first); @@ -294,7 +303,7 @@ ECTransaction::WritePlanObj::WritePlanObj( // We only need to update the parity buffer for the write for (auto && shard : sinfo.get_parity_shards()) { - will_write[shard] = truncate_write; + will_write[shard].insert(truncate_write); } } } @@ -1000,7 +1009,7 @@ void ECTransaction::Generate::appends_and_clone_ranges() { ECUtil::shard_extent_set_t cloneable_range(sinfo.get_k_plus_m()); sinfo.ro_size_to_read_mask(clone_max, cloneable_range); - if (plan.orig_size < plan.projected_size) { + if (op.delete_first || op.truncate || plan.orig_size < plan.projected_size) { ECUtil::shard_extent_set_t projected_cloneable_range(sinfo.get_k_plus_m()); sinfo.ro_size_to_read_mask(plan.projected_size,projected_cloneable_range);