osd: Fixes for osd_scrub_during_recovery handling

Fixes: http://tracker.ceph.com/issues/18206
Signed-off-by: David Zafman <dzafman@redhat.com>
(cherry picked from commit 367c32c69a512d2bea85a9b3860ec28bb4433750)

diff --git a/qa/standalone/scrub/osd-recovery-scrub.sh b/qa/standalone/scrub/osd-recovery-scrub.sh
new file mode 100755 (executable)
index 0000000..ef9a331
--- /dev/null
+++ b/qa/standalone/scrub/osd-recovery-scrub.sh
@@ -0,0 +1,129 @@
+#! /bin/bash
+#
+# Copyright (C) 2017 Red Hat <contact@redhat.com>
+#
+# Author: David Zafman <dzafman@redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Library Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Library Public License for more details.
+#
+source $CEPH_ROOT/qa/standalone/ceph-helpers.sh
+
+function run() {
+    local dir=$1
+    shift
+
+    export CEPH_MON="127.0.0.1:7124" # git grep '\<7124\>' : there must be only one
+    export CEPH_ARGS
+    CEPH_ARGS+="--fsid=$(uuidgen) --auth-supported=none "
+    CEPH_ARGS+="--mon-host=$CEPH_MON "
+
+    local funcs=${@:-$(set | sed -n -e 's/^\(TEST_[0-9a-z_]*\) .*/\1/p')}
+    for func in $funcs ; do
+        $func $dir || return 1
+    done
+}
+
+function TEST_recovery_scrub() {
+    local dir=$1
+    local poolname=test
+
+    TESTDATA="testdata.$$"
+    OSDS=8
+    PGS=32
+    OBJECTS=4
+
+    setup $dir || return 1
+    run_mon $dir a --osd_pool_default_size=1 || return 1
+    run_mgr $dir x || return 1
+    for osd in $(seq 0 $(expr $OSDS - 1))
+    do
+        run_osd $dir $osd || return 1
+    done
+
+    # Create a pool with $PGS pgs
+    create_pool $poolname $PGS $PGS
+    wait_for_clean || return 1
+    poolid=$(ceph osd dump | grep "^pool.*[']test[']" | awk '{ print $2 }')
+
+    dd if=/dev/urandom of=$TESTDATA bs=1M count=50
+    for i in $(seq 1 $OBJECTS)
+    do
+        rados -p $poolname put obj${i} $TESTDATA
+    done
+    rm -f $TESTDATA
+
+    ceph osd pool set $poolname size 4
+
+    pids=""
+    for pg in $(seq 0 $(expr $PGS - 1))
+    do
+        run_in_background pids pg_scrub $poolid.$(echo "{ obase=16; $pg }" | bc | tr '[:upper:]' '[:lower:]')
+    done
+    ceph pg dump pgs
+    wait_background pids
+    return_code=$?
+    if [ $return_code -ne 0 ]; then return $return_code; fi
+
+    ERRORS=0
+    pidfile=$(find $dir 2>/dev/null | grep $name_prefix'[^/]*\.pid')
+    pid=$(cat $pidfile)
+    if ! kill -0 $pid
+    then
+        echo "OSD crash occurred"
+        tail -100 $dir/osd.0.log
+        ERRORS=$(expr $ERRORS + 1)
+    fi
+
+    kill_daemons $dir || return 1
+
+    declare -a err_strings
+    err_strings[0]="not scheduling scrubs due to active recovery"
+    # Test with these two strings after disabled check in OSD::sched_scrub()
+    #err_strings[0]="handle_scrub_reserve_request: failed to reserve remotely"
+    #err_strings[1]="sched_scrub: failed to reserve locally"
+
+    for osd in $(seq 0 $(expr $OSDS - 1))
+    do
+        grep "failed to reserve\|not scheduling scrubs" $dir/osd.${osd}.log
+    done
+    for err_string in "${err_strings[@]}"
+    do
+        found=false
+        for osd in $(seq 0 $(expr $OSDS - 1))
+        do
+            if grep "$err_string" $dir/osd.${osd}.log > /dev/null;
+            then
+                found=true
+            fi
+        done
+        if [ "$found" = "false" ]; then
+            echo "Missing log message '$err_string'"
+           ERRORS=$(expr $ERRORS + 1)
+        fi
+    done
+
+    teardown $dir || return 1
+
+    if [ $ERRORS != "0" ];
+    then
+        echo "TEST FAILED WITH $ERRORS ERRORS"
+        return 1
+    fi
+
+    echo "TEST PASSED"
+    return 0
+}
+
+main osd-recovery-scrub "$@"
+
+# Local Variables:
+# compile-command: "cd build ; make -j4 && \
+#    ../qa/run-standalone.sh osd-recovery-scrub.sh"

diff --git a/src/common/AsyncReserver.h b/src/common/AsyncReserver.h
index ae22b535d7360ab694bc0ced6187637a85ee6459..28512ac800e374804349dddb326ea5bb881586a7 100644 (file)
--- a/src/common/AsyncReserver.h
+++ b/src/common/AsyncReserver.h
@@ -144,6 +144,16 @@ public:
     }
     do_queues();
   }
+
+  /**
+   * Has reservations
+   *
+   * Return true if there are reservations in progress
+   */
+  bool has_reservation() {
+    Mutex::Locker l(lock);
+    return !in_progress.empty();
+  }
   static const unsigned MAX_PRIORITY = (unsigned)-1;
 };
 

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index afbfe68a22e2f3f6f95e8b4dd953072dc5b95bd3..68d179208177afa147741f43bdbdc77c8841f564 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7348,6 +7348,11 @@ void OSD::sched_scrub()
   if (!service.can_inc_scrubs_pending()) {
     return;
   }
+  if (!cct->_conf->osd_scrub_during_recovery && service.is_recovery_active()) {
+    dout(20) << __func__ << " not scheduling scrubs due to active recovery" << dendl;
+    return;
+  }
+
 
   utime_t now = ceph_clock_now();
   bool time_permit = scrub_time_permit(now);
@@ -7366,11 +7371,6 @@ void OSD::sched_scrub()
        break;
       }
 
-      if (!cct->_conf->osd_scrub_during_recovery && service.is_recovery_active()) {
-        dout(10) << __func__ << "not scheduling scrub of " << scrub.pgid << " due to active recovery ops" << dendl;
-        break;
-      }
-
       if ((scrub.deadline >= now) && !(time_permit && load_is_low)) {
         dout(10) << __func__ << " not scheduling scrub for " << scrub.pgid << " due to "
                  << (!time_permit ? "time not permit" : "high load") << dendl;
@@ -9446,8 +9446,7 @@ void OSDService::finish_recovery_op(PG *pg, const hobject_t& soid, bool dequeue)
 
 bool OSDService::is_recovery_active()
 {
-  Mutex::Locker l(recovery_lock);
-  return recovery_ops_active > 0;
+  return local_reserver.has_reservation() || remote_reserver.has_reservation();
 }
 
 // =========================================================

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index e0707efe2d581bf8ce1ca81ff5c3ab2391e8c01c..6bd38e4165ad670a3ad260c077ee45a2de46679e 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -3611,13 +3611,14 @@ bool PG::sched_scrub()
   bool ret = true;
   if (!scrubber.reserved) {
     assert(scrubber.reserved_peers.empty());
-    if (osd->inc_scrubs_pending()) {
-      dout(20) << "sched_scrub: reserved locally, reserving replicas" << dendl;
+    if ((cct->_conf->osd_scrub_during_recovery || !osd->is_recovery_active()) &&
+         osd->inc_scrubs_pending()) {
+      dout(20) << __func__ << ": reserved locally, reserving replicas" << dendl;
       scrubber.reserved = true;
       scrubber.reserved_peers.insert(pg_whoami);
       scrub_reserve_replicas();
     } else {
-      dout(20) << "sched_scrub: failed to reserve locally" << dendl;
+      dout(20) << __func__ << ": failed to reserve locally" << dendl;
       ret = false;
     }
   }
@@ -3797,7 +3798,13 @@ void PG::handle_scrub_reserve_request(OpRequestRef op)
             << dendl;
     return;
   }
-  scrubber.reserved = osd->inc_scrubs_pending();
+  if ((cct->_conf->osd_scrub_during_recovery || !osd->is_recovery_active()) &&
+      osd->inc_scrubs_pending()) {
+    scrubber.reserved = true;
+  } else {
+    dout(20) << __func__ << ": failed to reserve remotely" << dendl;
+    scrubber.reserved = false;
+  }
   if (op->get_req()->get_type() == MSG_OSD_SCRUB_RESERVE) {
     const MOSDScrubReserve *m =
       static_cast<const MOSDScrubReserve*>(op->get_req());