Merge pull request ceph#64564 from JonBailey1993/ceph_test_rados_io_sequence_set_optimisations_earlier

test/osd: Move initialisation of overwrites and optimisation earlier in ceph_test_rados_io_sequence

Reviewed-by: Ronen Friedman <[email protected]>

Author: JonBailey1993

src/common/io_exerciser/RadosIo.cc

@@ -63,10 +63,6 @@ RadosIo::RadosIo(librados::Rados& rados, boost::asio::io_context& asio,
   int rc;
   rc = rados.ioctx_create(pool.c_str(), io);
   ceph_assert(rc == 0);
-  allow_ec_overwrites(true);
-  if (ec_optimizations) {
-    allow_ec_optimizations();
-  }
 }
 
 RadosIo::~RadosIo() {}
@@ -90,28 +86,6 @@ void RadosIo::wait_for_io(int count) {
   }
 }
 
-void RadosIo::allow_ec_overwrites(bool allow) {
-  int rc;
-  bufferlist inbl, outbl;
-  std::string cmdstr = "{\"prefix\": \"osd pool set\", \"pool\": \"" + pool +
-                       "\", \
-      \"var\": \"allow_ec_overwrites\", \"val\": \"" +
-                       (allow ? "true" : "false") + "\"}";
-  rc = rados.mon_command(cmdstr, inbl, &outbl, nullptr);
-  ceph_assert(rc == 0);
-}
-
-void RadosIo::allow_ec_optimizations()
-{
-  int rc;
-  bufferlist inbl, outbl;
-  std::string cmdstr =
-    "{\"prefix\": \"osd pool set\", \"pool\": \"" + pool + "\", \
-      \"var\": \"allow_ec_optimizations\", \"val\": \"true\"}";
-  rc = rados.mon_command(cmdstr, inbl, &outbl, nullptr);
-  ceph_assert(rc == 0);
-}
-
 template <int N>
 RadosIo::AsyncOpInfo<N>::AsyncOpInfo(const std::array<uint64_t, N>& offset,
                                      const std::array<uint64_t, N>& length)
@@ -197,8 +171,14 @@ void RadosIo::applyIoOp(IoOp& op) {
 
     case OpType::Consistency: {
       start_io();
+      ceph_assert(cc);
       bool is_consistent =
-          cc->single_read_and_check_consistency(oid, block_size, 0, 0);
+        cc->single_read_and_check_consistency(oid, block_size, 0, 0);
+      if (!is_consistent) {
+        std::stringstream strstream;
+        cc->print_results(strstream);
+        std::cerr << strstream.str() << std::endl;
+      }
       ceph_assert(is_consistent);
       finish_io();
       break;

src/common/json/OSDStructures.cc

@@ -69,6 +69,21 @@ void OSDPoolGetReply::decode_json(JSONObj* obj) {
   JSONDecoder::decode_json("allow_ec_optimizations", allow_ec_optimizations, obj);
 }
 
+void OSDPoolSetRequest::dump(Formatter* f) const {
+  encode_json("prefix", "osd pool set", f);
+  encode_json("pool", pool, f);
+  encode_json("var", var, f);
+  encode_json("val", val, f);
+  encode_json("yes_i_really_mean_it", yes_i_really_mean_it, f);
+}
+
+void OSDPoolSetRequest::decode_json(JSONObj* obj) {
+  JSONDecoder::decode_json("pool", pool, obj);
+  JSONDecoder::decode_json("var", var, obj);
+  JSONDecoder::decode_json("val", val, obj);
+  JSONDecoder::decode_json("yes_i_really_mean_it", yes_i_really_mean_it, obj);
+}
+
 void OSDECProfileGetRequest::dump(Formatter* f) const {
   encode_json("prefix", "osd erasure-code-profile get", f);
   encode_json("name", name, f);

src/common/json/OSDStructures.h

@@ -55,6 +55,16 @@ struct OSDPoolGetReply {
   void decode_json(JSONObj* obj);
 };
 
+struct OSDPoolSetRequest {
+  std::string pool;
+  std::string var;
+  std::optional<std::string> val;
+  std::optional<bool> yes_i_really_mean_it = std::nullopt;
+
+  void dump(Formatter* f) const;
+  void decode_json(JSONObj* obj);
+};
+
 struct OSDECProfileGetRequest {
   std::string name;
   std::string format = "json";

src/erasure-code/consistency/ConsistencyChecker.cc

@@ -5,6 +5,7 @@
 #include "ECReader.h"
 #include "ECEncoder.h"
 #include "ECEncoderSwitch.h"
+#include "osd/ECUtil.h"
 
 using ConsistencyChecker = ceph::consistency::ConsistencyChecker;
 
@@ -115,6 +116,30 @@ bool ConsistencyChecker::check_object_consistency(const std::string& oid,
     return false;
   }
 
+  // Encode always returns 4k aligned data
+  // A client read will always return parity with the same size as shard 0
+  // We confirm the encoded data is the same or larger than the client read
+  ceph_assert(outbl->length() >= data_and_parity.second.length());
+  // We check the difference is not larger than the page size multipled by the
+  //    number of parities
+  ceph_assert(outbl->length() - data_and_parity.second.length()
+                <= (encoder.get_m() * encoder.get_chunk_size()) - encoder.get_m());
+  // We truncate the encoded parity to the size of the client read for comparison
+  if (outbl->length() != data_and_parity.second.length())
+  {
+    const int aligned_shard_length = outbl->length() / encoder.get_m();
+    const int shard_data_length = data_and_parity.second.length()
+                                  / encoder.get_m();
+    ceph::bufferlist newdata;
+    for (int i = 0; i < encoder.get_m(); i++)
+    {
+      ceph::bufferlist bl;
+      bl.substr_of(*outbl, i * aligned_shard_length, shard_data_length);
+      newdata.append(bl);
+    }
+    outbl = std::move(newdata);
+  }
+
   return buffers_match(outbl.value(), data_and_parity.second);
 }
 

src/erasure-code/consistency/ECEncoder.cc

@@ -198,6 +198,17 @@ int ECEncoder<SInfo>::get_m()
 {
   return stripe_info->get_m();
 }
+
+/**
+ * Return chunksize for the stripe
+ *
+ * @returns int Chunksize for the stripe
+ */
+template <typename SInfo>
+int ECEncoder<SInfo>::get_chunk_size()
+{
+  return stripe_info->get_chunk_size();
+}
 }
 }
 

src/erasure-code/consistency/ECEncoder.h

@@ -29,6 +29,7 @@ class ECEncoder {
     std::optional<ceph::bufferlist> do_encode(ceph::bufferlist inbl);
     int get_k(void);
     int get_m(void);
+    int get_chunk_size(void);
 };
 }
 }

src/erasure-code/consistency/ECEncoderSwitch.cc

@@ -57,3 +57,17 @@ int ECEncoderSwitch::get_m()
     return encoder_legacy.get_m();
   }
 }
+
+/**
+ * Return chunksize for the stripe from the correct version of the encoder
+ *
+ * @returns int Chunksize for stripe
+ */
+int ECEncoderSwitch::get_chunk_size()
+{
+  if (optimizations_enabled) {
+    return encoder_optimized.get_chunk_size();
+  } else {
+    return encoder_legacy.get_chunk_size();
+  }
+}

src/erasure-code/consistency/ECEncoderSwitch.h

@@ -26,6 +26,7 @@ class ECEncoderSwitch {
     std::optional<ceph::bufferlist> do_encode(ceph::bufferlist inbl);
     int get_k(void);
     int get_m(void);
+    int get_chunk_size(void);
 };
 }
 }

src/test/osd/ceph_test_rados_io_sequence/ceph_test_rados_io_sequence.cc

@@ -794,7 +794,7 @@ ceph::io_sequence::tester::SelectErasurePool::SelectErasurePool(
     bool allow_pool_deep_scrubbing,
     bool allow_pool_scrubbing,
     bool test_recovery,
-    bool disable_pool_ec_optimizations)
+    bool allow_pool_ec_optimizations)
     : ProgramOptionReader<std::string>(vm, "pool"),
       rados(rados),
       dry_run(dry_run),
@@ -803,7 +803,7 @@ ceph::io_sequence::tester::SelectErasurePool::SelectErasurePool(
       allow_pool_deep_scrubbing(allow_pool_deep_scrubbing),
       allow_pool_scrubbing(allow_pool_scrubbing),
       test_recovery(test_recovery),
-      disable_pool_ec_optimizations(disable_pool_ec_optimizations),
+      allow_pool_ec_optimizations(allow_pool_ec_optimizations),
       first_use(true),
       sep{cct, rng, vm, rados, dry_run, first_use} {
   if (isForced()) {
@@ -849,9 +849,10 @@ const std::string ceph::io_sequence::tester::SelectErasurePool::select() {
     }
 
     if (!dry_run) {
-      configureServices(allow_pool_autoscaling, allow_pool_balancer,
+      configureServices(force_value.value_or(created_pool_name),
+                        allow_pool_autoscaling, allow_pool_balancer,
                         allow_pool_deep_scrubbing, allow_pool_scrubbing,
-                        test_recovery);
+                        allow_pool_ec_optimizations, true, test_recovery);
 
       setApplication(created_pool_name);
     }
@@ -868,7 +869,7 @@ std::string ceph::io_sequence::tester::SelectErasurePool::create() {
   std::string pool_name;
   profile = sep.select();
   pool_name = fmt::format("testpool-pr{}{}", profile->name,
-    disable_pool_ec_optimizations?"_no_ec_opt":"");
+    allow_pool_ec_optimizations?"":"_no_ec_opt");
 
   ceph::messaging::osd::OSDECPoolCreateRequest pool_create_request{
       pool_name, "erasure", 8, 8, profile->name};
@@ -895,10 +896,13 @@ void ceph::io_sequence::tester::SelectErasurePool::setApplication(
 }
 
 void ceph::io_sequence::tester::SelectErasurePool::configureServices(
+    const std::string& pool_name,
     bool allow_pool_autoscaling,
     bool allow_pool_balancer,
     bool allow_pool_deep_scrubbing,
     bool allow_pool_scrubbing,
+    bool allow_pool_ec_optimizations,
+    bool allow_pool_ec_overwrites,
     bool test_recovery) {
   int rc;
   bufferlist inbl, outbl;
@@ -942,12 +946,34 @@ void ceph::io_sequence::tester::SelectErasurePool::configureServices(
 
   if (!allow_pool_scrubbing) {
     ceph::messaging::osd::OSDSetRequest no_scrub_request{"noscrub",
-                                                          std::nullopt};
+                                                         std::nullopt};
     rc = send_mon_command(no_scrub_request, rados, "OSDSetRequest", inbl,
                           &outbl, formatter.get());
     ceph_assert(rc == 0);
   }
 
+  if (allow_pool_ec_optimizations) {
+    ceph::messaging::osd::OSDPoolSetRequest
+        allow_ec_optimisations_request{pool_name,
+                                       "allow_ec_optimizations",
+                                       "true",
+                                       std::nullopt};
+    rc = send_mon_command(allow_ec_optimisations_request, rados,
+                          "OSDPoolSetRequest", inbl, &outbl, formatter.get());
+    ceph_assert(rc == 0);
+  }
+
+  if (allow_pool_ec_overwrites) {
+    ceph::messaging::osd::OSDPoolSetRequest
+        allow_ec_optimisations_request{pool_name,
+                                       "allow_ec_overwrites",
+                                       "true",
+                                       std::nullopt};
+    rc = send_mon_command(allow_ec_optimisations_request, rados,
+                          "OSDPoolSetRequest", inbl, &outbl, formatter.get());
+    ceph_assert(rc == 0);
+  }
+
   if (test_recovery) {
     ceph::messaging::config::ConfigSetRequest bluestore_debug_request{
         "global", "bluestore_debug_inject_read_err", "true", std::nullopt};
@@ -1105,7 +1131,7 @@ ceph::io_sequence::tester::TestRunner::TestRunner(
           vm.contains("allow_pool_deep_scrubbing"),
           vm.contains("allow_pool_scrubbing"),
           vm.contains("testrecovery"),
-          vm.contains("disable_pool_ec_optimizations")},
+          !vm.contains("disable_pool_ec_optimizations")},
       snt{rng, vm, "threads", true},
       ssr{vm} {
   dout(0) << "Test using seed " << seed << dendl;
@@ -1127,7 +1153,6 @@ ceph::io_sequence::tester::TestRunner::TestRunner(
   allow_pool_balancer = vm.contains("allow_pool_balancer");
   allow_pool_deep_scrubbing = vm.contains("allow_pool_deep_scrubbing");
   allow_pool_scrubbing = vm.contains("allow_pool_scrubbing");
-  disable_pool_ec_optimizations = vm.contains("disable_pool_ec_optimizations");
 
   if (testrecovery && (num_objects > 1)) {
     throw std::invalid_argument("testrecovery option not allowed if parallel is"

src/test/osd/ceph_test_rados_io_sequence/ceph_test_rados_io_sequence.h

@@ -389,7 +389,7 @@ class SelectErasurePool : public ProgramOptionReader<std::string> {
                     bool allow_pool_deep_scrubbing,
                     bool allow_pool_scrubbing,
                     bool test_recovery,
-                    bool disable_pool_ec_optimizations);
+                    bool allow_pool_ec_optimizations);
   const std::string select() override;
   std::string create();
 
@@ -400,7 +400,7 @@ class SelectErasurePool : public ProgramOptionReader<std::string> {
   }
   inline bool get_allow_pool_scrubbing() { return allow_pool_scrubbing; }
   inline bool get_allow_pool_ec_optimizations() {
-    return !disable_pool_ec_optimizations;
+    return allow_pool_ec_optimizations;
   }
   inline std::optional<Profile> getProfile() { return profile; }
 
@@ -413,18 +413,21 @@ class SelectErasurePool : public ProgramOptionReader<std::string> {
   bool allow_pool_deep_scrubbing;
   bool allow_pool_scrubbing;
   bool test_recovery;
-  bool disable_pool_ec_optimizations;
+  bool allow_pool_ec_optimizations;
 
   bool first_use;
 
   SelectErasureProfile sep;
 
   std::optional<Profile> profile;
 
-  void configureServices(bool allow_pool_autoscaling,
+  void configureServices(const std::string& pool_name,
+                         bool allow_pool_autoscaling,
                          bool allow_pool_balancer,
                          bool allow_pool_deep_scrubbing,
                          bool allow_pool_scrubbing,
+                         bool disable_pool_ec_optimizations,
+                         bool allow_pool_ec_overwrites,
                          bool test_recovery);
 
  void setApplication(const std::string& pool_name);
@@ -515,7 +518,6 @@ class TestRunner {
   bool allow_pool_balancer;
   bool allow_pool_deep_scrubbing;
   bool allow_pool_scrubbing;
-  bool disable_pool_ec_optimizations;
 
   bool show_sequence;
   bool show_help;