]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
osd/ECTransaction: fix truncate+write planning for EC shard sizes 69663/head
authorAlex Ainscow <aainscow@uk.ibm.com>
Tue, 9 Jun 2026 14:45:25 +0000 (15:45 +0100)
committerAlex Ainscow <aainscow@uk.ibm.com>
Tue, 23 Jun 2026 08:06:35 +0000 (09:06 +0100)
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 <aainscow@uk.ibm.com>
Assisted-by: IBM-Bob:ClaudeSonnet/GPT
src/osd/ECTransaction.cc

index 8368a219040465dfd9ff1d9c56b6ae294011203f..ed6551b711aba7a6169840861cccf69ebd8075b9 100644 (file)
@@ -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);