From 367c32c69a512d2bea85a9b3860ec28bb4433750 Mon Sep 17 00:00:00 2001 From: David Zafman Date: Tue, 15 Aug 2017 14:45:13 -0700 Subject: [PATCH] osd: Fixes for osd_scrub_during_recovery handling Fixes: http://tracker.ceph.com/issues/18206 Signed-off-by: David Zafman --- qa/standalone/scrub/osd-recovery-scrub.sh | 129 ++++++++++++++++++++++ src/common/AsyncReserver.h | 10 ++ src/osd/OSD.cc | 13 +-- src/osd/PG.cc | 15 ++- 4 files changed, 156 insertions(+), 11 deletions(-) create mode 100755 qa/standalone/scrub/osd-recovery-scrub.sh diff --git a/qa/standalone/scrub/osd-recovery-scrub.sh b/qa/standalone/scrub/osd-recovery-scrub.sh new file mode 100755 index 00000000000..ef9a3318afd --- /dev/null +++ b/qa/standalone/scrub/osd-recovery-scrub.sh @@ -0,0 +1,129 @@ +#! /bin/bash +# +# Copyright (C) 2017 Red Hat +# +# Author: David Zafman +# +# 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 ae22b535d73..28512ac800e 100644 --- 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 f35415fd2ce..7cafff17a47 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -7373,6 +7373,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); @@ -7391,11 +7396,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; @@ -9471,8 +9471,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 371e6380800..5849c64bb07 100644 --- 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(op->get_req()); -- 2.39.5