From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Tue, 23 Jul 2024 13:04:57 +0000 (+0000)
Subject: osd: fix partial reading during multi-region EC reads
X-Git-Tag: v20.0.0~933^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=beb4d2286ac7c568afc9e8fb8eee42767305cbf5;p=ceph.git

osd: fix partial reading during multi-region EC reads

Fixes: https://tracker.ceph.com/issues/67087
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/osd/ECCommon.cc b/src/osd/ECCommon.cc
index 02bb04c4a0a7..1fc876105029 100644
--- a/src/osd/ECCommon.cc
+++ b/src/osd/ECCommon.cc
@@ -327,15 +327,14 @@ void ECCommon::ReadPipeline::get_min_want_to_read_shards(
 {
   const auto [left_chunk_index, right_chunk_index] =
     sinfo.offset_length_to_data_chunk_indices(offset, length);
-  for(uint64_t i = left_chunk_index; i < right_chunk_index; i++) {
-    auto raw_chunk = i % sinfo.get_data_chunk_count();
+  const auto distance =
+    std::min(right_chunk_index - left_chunk_index,
+             sinfo.get_data_chunk_count());
+  for(uint64_t i = 0; i < distance; i++) {
+    auto raw_chunk = (left_chunk_index + i) % sinfo.get_data_chunk_count();
     auto chunk = chunk_mapping.size() > raw_chunk ?
       chunk_mapping[raw_chunk] : static_cast<int>(raw_chunk);
-    if (auto [_, inserted] = want_to_read->insert(chunk); !inserted) {
-      // aready processed all chunks
-      ceph_assert(want_to_read->size() == sinfo.get_data_chunk_count());
-      break;
-    }
+    want_to_read->insert(chunk);
   }
 }
 
diff --git a/src/test/osd/TestECBackend.cc b/src/test/osd/TestECBackend.cc
index d28d428fc060..f93ed7ff67a8 100644
--- a/src/test/osd/TestECBackend.cc
+++ b/src/test/osd/TestECBackend.cc
@@ -230,3 +230,28 @@ TEST(ECCommon, get_min_want_to_read_shards)
     ASSERT_TRUE(want_to_read == (std::set<int>{0, 1, 2, 3}));
   }
 }
+
+TEST(ECCommon, get_min_want_to_read_shards_bug67087)
+{
+  const uint64_t swidth = 4096;
+  const uint64_t ssize = 4;
+
+  ECUtil::stripe_info_t s(ssize, swidth);
+  ASSERT_EQ(s.get_stripe_width(), swidth);
+  ASSERT_EQ(s.get_chunk_size(), 1024);
+
+  const std::vector<int> chunk_mapping = {}; // no remapping
+
+  std::set<int> want_to_read;
+
+  // multitple calls with the same want_to_read can happen during
+  // multi-region reads.
+  {
+    ECCommon::ReadPipeline::get_min_want_to_read_shards(
+      512, 512, s, chunk_mapping, &want_to_read);
+    ASSERT_EQ(want_to_read, std::set<int>{0});
+    ECCommon::ReadPipeline::get_min_want_to_read_shards(
+      512+16*1024, 512, s, chunk_mapping, &want_to_read);
+    ASSERT_EQ(want_to_read, std::set<int>{0});
+  }
+}