Merge pull request ceph#60872 from ronen-fr/wip-rf-repaironly

osd/scrub: remove config option osd_repair_during_recovery
Reviewed-by: Samuel Just <[email protected]>

Author: ronen-fr

qa/standalone/scrub/osd-scrub-repair.sh

@@ -107,6 +107,30 @@ function TEST_corrupt_and_repair_replicated() {
     corrupt_and_repair_one $dir $poolname $(get_primary $poolname SOMETHING) || return 1
 }
 
+#
+# Allow operator-initiated scrubs to be scheduled even when some recovering is still
+# undergoing on the same OSD
+#
+function TEST_allow_oper_initiated_scrub_during_recovery() {
+    local dir=$1
+    local -A cluster_conf=(
+        ['osds_num']="2"
+        ['pgs_in_pool']="4"
+        ['pool_name']="nopool"
+        ['pool_default_size']="2"
+        ['extras']="--osd_scrub_during_recovery=false \
+                    --osd_debug_pretend_recovery_active=true"
+    )
+
+    standard_scrub_cluster $dir cluster_conf
+    local poolname=rbd
+    create_rbd_pool || return 1
+    wait_for_clean || return 1
+
+    add_something $dir $poolname || return 1
+    oper_scrub_and_schedule $dir $poolname $(get_not_primary $poolname SOMETHING) || return 1
+}
+
 #
 # Allow repair to be scheduled when some recovering is still undergoing on the same OSD
 #
@@ -117,10 +141,8 @@ function TEST_allow_repair_during_recovery() {
     run_mon $dir a --osd_pool_default_size=2 || return 1
     run_mgr $dir x || return 1
     run_osd $dir 0 --osd_scrub_during_recovery=false \
-                   --osd_repair_during_recovery=true \
                    --osd_debug_pretend_recovery_active=true || return 1
     run_osd $dir 1 --osd_scrub_during_recovery=false \
-                   --osd_repair_during_recovery=true \
                    --osd_debug_pretend_recovery_active=true || return 1
     create_rbd_pool || return 1
     wait_for_clean || return 1
@@ -132,25 +154,62 @@ function TEST_allow_repair_during_recovery() {
 #
 # Skip non-repair scrub correctly during recovery
 #
+# Note: forgoing the automatic creation of a pool in standard_scrub_cluster as
+#       the test requires a specific RBD pool.
 function TEST_skip_non_repair_during_recovery() {
     local dir=$1
-    local poolname=rbd
+    local -A cluster_conf=(
+        ['osds_num']="2"
+        ['pgs_in_pool']="4"
+        ['pool_name']="nopool"
+        ['pool_default_size']="2"
+        ['extras']="--osd_scrub_during_recovery=false --osd_debug_pretend_recovery_active=true"
+    )
 
-    run_mon $dir a --osd_pool_default_size=2 || return 1
-    run_mgr $dir x || return 1
-    run_osd $dir 0 --osd_scrub_during_recovery=false \
-                   --osd_repair_during_recovery=true \
-                   --osd_debug_pretend_recovery_active=true || return 1
-    run_osd $dir 1 --osd_scrub_during_recovery=false \
-                   --osd_repair_during_recovery=true \
-                   --osd_debug_pretend_recovery_active=true || return 1
+    standard_scrub_cluster $dir cluster_conf
+    local poolname=rbd
     create_rbd_pool || return 1
     wait_for_clean || return 1
 
     add_something $dir $poolname || return 1
     scrub_and_not_schedule $dir $poolname $(get_not_primary $poolname SOMETHING) || return 1
 }
 
+
+function oper_scrub_and_schedule() {
+    local dir=$1
+    local poolname=$2
+    local osd=$3
+
+    #
+    # 1) start an operator-initiated scrub
+    #
+    local pg=$(get_pg $poolname SOMETHING)
+    local last_scrub=$(get_last_scrub_stamp $pg)
+    ceph tell $pg scrub
+
+    #
+    # 2) Assure the scrub was executed
+    #
+    sleep 3
+    for ((i=0; i < 3; i++)); do
+        if test "$(get_last_scrub_stamp $pg)" '>' "$last_scrub" ; then
+            break
+        fi
+        if test "$(get_last_scrub_stamp $pg)" '==' "$last_scrub" ; then
+            return 1
+        fi
+        sleep 1
+    done
+
+    #
+    # 3) Access to the file must OK
+    #
+    objectstore_tool $dir $osd SOMETHING list-attrs || return 1
+    rados --pool $poolname get SOMETHING $dir/COPY || return 1
+    diff $dir/ORIGINAL $dir/COPY || return 1
+}
+
 function scrub_and_not_schedule() {
     local dir=$1
     local poolname=$2
@@ -166,6 +225,7 @@ function scrub_and_not_schedule() {
     #
     # 2) Assure the scrub is not scheduled
     #
+    sleep 3
     for ((i=0; i < 3; i++)); do
         if test "$(get_last_scrub_stamp $pg)" '>' "$last_scrub" ; then
             return 1
@@ -218,9 +278,9 @@ function corrupt_and_repair_two() {
 
 #
 # 1) add an object
-# 2) remove the corresponding file from a designated OSD
+# 2) remove the corresponding object from a designated OSD
 # 3) repair the PG
-# 4) check that the file has been restored in the designated OSD
+# 4) check that the object has been restored in the designated OSD
 #
 function corrupt_and_repair_one() {
     local dir=$1

qa/standalone/scrub/scrub-helpers.sh

@@ -231,16 +231,21 @@ function standard_scrub_cluster() {
 
     local OSDS=${args['osds_num']:-"3"}
     local pg_num=${args['pgs_in_pool']:-"8"}
+
     local poolname="${args['pool_name']:-test}"
     args['pool_name']=$poolname
+
+    local pool_default_size=${args['pool_default_size']:-3}
+    args['pool_default_size']=$pool_default_size
+
     local extra_pars=${args['extras']}
     local debug_msg=${args['msg']:-"dbg"}
 
     # turn off '-x' (but remember previous state)
     local saved_echo_flag=${-//[^x]/}
     set +x
 
-    run_mon $dir a --osd_pool_default_size=3 || return 1
+    run_mon $dir a --osd_pool_default_size=$pool_default_size || return 1
     run_mgr $dir x --mgr_stats_period=1 || return 1
 
     local ceph_osd_args="--osd_deep_scrub_randomize_ratio=0 \
@@ -254,22 +259,26 @@ function standard_scrub_cluster() {
             --osd_scrub_retry_after_noscrub=5 \
             --osd_scrub_retry_pg_state=5 \
             --osd_scrub_retry_delay=3 \
-            --osd_pool_default_size=3 \
+            --osd_pool_default_size=$pool_default_size \
             $extra_pars"
 
     for osd in $(seq 0 $(expr $OSDS - 1))
     do
       run_osd $dir $osd $(echo $ceph_osd_args) || return 1
     done
 
-    create_pool $poolname $pg_num $pg_num
-    wait_for_clean || return 1
+    if [[ "$poolname" != "nopool" ]]; then
+        create_pool $poolname $pg_num $pg_num
+        wait_for_clean || return 1
+    fi
 
     # update the in/out 'args' with the ID of the new pool
     sleep 1
-    name_n_id=`ceph osd dump | awk '/^pool.*'$poolname'/ { gsub(/'"'"'/," ",$3); print $3," ", $2}'`
-    echo "standard_scrub_cluster: $debug_msg: test pool is $name_n_id"
-    args['pool_id']="${name_n_id##* }"
+    if [[ "$poolname" != "nopool" ]]; then
+        name_n_id=`ceph osd dump | awk '/^pool.*'$poolname'/ { gsub(/'"'"'/," ",$3); print $3," ", $2}'`
+        echo "standard_scrub_cluster: $debug_msg: test pool is $name_n_id"
+        args['pool_id']="${name_n_id##* }"
+    fi
     args['osd_args']=$ceph_osd_args
     if [[ -n "$saved_echo_flag" ]]; then set -x; fi
 }

src/common/options/osd.yaml.in

@@ -211,12 +211,6 @@ options:
   level: advanced
   desc: Asserts that no clone-objects were added to a snap after we start trimming it
   default: false
-- name: osd_repair_during_recovery
-  type: bool
-  level: advanced
-  desc: Allow requested repairing when PGs on the OSD are undergoing recovery
-  default: false
-  with_legacy: true
 - name: osd_scrub_begin_hour
   type: int
   level: advanced

src/osd/scrubber/osd_scrub.cc

@@ -201,18 +201,9 @@ Scrub::OSDRestrictions OsdScrub::restrictions_on_scrubbing(
     env_conditions.random_backoff_active = true;
 
   } else if (is_recovery_active && !conf->osd_scrub_during_recovery) {
-    if (conf->osd_repair_during_recovery) {
-      dout(15)
-	  << "will only schedule explicitly requested repair due to active "
-	     "recovery"
-	  << dendl;
-      env_conditions.allow_requested_repair_only = true;
-
-    } else {
-      dout(15) << "recovery in progress. Operator-initiated scrubs only."
-	       << dendl;
-      env_conditions.recovery_in_progress = true;
-    }
+    dout(15) << "recovery in progress. Operator-initiated scrubs only."
+	     << dendl;
+    env_conditions.recovery_in_progress = true;
   } else {
 
     // regular, i.e. non-high-priority scrubs are allowed

src/osd/scrubber/pg_scrubber.cc

@@ -2256,20 +2256,6 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
     }
   }
 
-  // if only explicitly requested repairing is allowed - skip other types
-  // of scrubbing
-  if (osd_restrictions.allow_requested_repair_only &&
-      ScrubJob::observes_recovery(trgt.urgency())) {
-    dout(10) << __func__
-	     << ": skipping this PG as repairing was not explicitly "
-		"requested for it"
-	     << dendl;
-    requeue_penalized(
-	s_or_d, delay_both_targets_t::yes, delay_cause_t::scrub_params,
-	clock_now);
-    return schedule_result_t::target_specific_failure;
-  }
-
   // try to reserve the local OSD resources. If failing: no harm. We will
   // be retried by the OSD later on.
   if (!reserve_local(trgt)) {

src/osd/scrubber_common.h

@@ -89,17 +89,13 @@ struct OSDRestrictions {
   /// rolled a dice, and decided not to scrub in this tick
   bool random_backoff_active{false};
 
-  /// the OSD is performing recovery & osd_repair_during_recovery is 'true'
-  bool allow_requested_repair_only:1{false};
-
   /// the CPU load is high. No regular scrubs are allowed.
   bool cpu_overloaded:1{false};
 
   /// outside of allowed scrubbing hours/days
   bool restricted_time:1{false};
 
-  /// the OSD is performing a recovery, osd_scrub_during_recovery is 'false',
-  /// and so is osd_repair_during_recovery
+  /// the OSD is performing a recovery & osd_scrub_during_recovery is 'false'
   bool recovery_in_progress:1{false};
 };
 static_assert(sizeof(Scrub::OSDRestrictions) <= sizeof(uint32_t));
@@ -193,13 +189,12 @@ struct formatter<Scrub::OSDRestrictions> {
   auto format(const Scrub::OSDRestrictions& conds, FormatContext& ctx) const
   {
     return fmt::format_to(
-	ctx.out(), "<{}.{}.{}.{}.{}.{}>",
+	ctx.out(), "<{}.{}.{}.{}.{}>",
 	conds.max_concurrency_reached ? "max-scrubs" : "",
 	conds.random_backoff_active ? "backoff" : "",
 	conds.cpu_overloaded ? "high-load" : "",
 	conds.restricted_time ? "time-restrict" : "",
-	conds.recovery_in_progress ? "recovery" : "",
-	conds.allow_requested_repair_only ? "repair-only" : "");
+	conds.recovery_in_progress ? "recovery" : "");
   }
 };