os: FileStore::lfn_unlink always clears FDCache

Otherwise the FDCache will keep a file descriptor to a file that was
removed from the file system. This may create various type of errors
because the OSD checking the FDCache will assume the file that contains
information for an object exists although it does not. For instance in
the following:

      * rados put object file
      * rm file from the primary
      * repair the pg to which the object is mapped

if the FDCache is not cleared, repair will incorrectly pull a copy from
a replica and write it to the now unlinked file. Later on, it will
assume the file exists on the primary and only be partially correct :
the data can still be accessed via the file descriptor but any operation
using the path name will fail.

http://tracker.ceph.com/issues/8914 Fixes: #8914

Signed-off-by: Loic Dachary <loic-201408@dachary.org>

diff --git a/src/common/shared_cache.hpp b/src/common/shared_cache.hpp
index f5f53f90aec621506fb49f7cf2789faedc7b3d3f..a5079ede6a0ca096385a66edd211683c987fd5c3 100644 (file)
--- a/src/common/shared_cache.hpp
+++ b/src/common/shared_cache.hpp
@@ -103,6 +103,12 @@ public:
     cct = c;
   }
 
+  void dump_weak_refs() {
+    lderr(cct) << "leaked refs:\n";
+    dump_weak_refs(*_dout);
+    *_dout << dendl;
+  }
+
   void dump_weak_refs(ostream& out) {
     for (typename map<K, WeakVPtr>::iterator p = weak_refs.begin();
         p != weak_refs.end();

diff --git a/src/os/FDCache.h b/src/os/FDCache.h
index 54f2411fab68a2da3824c8a736e8b130b132acfb..5b06e34afac5a8e89220f042e4305481c3067a2e 100644 (file)
--- a/src/os/FDCache.h
+++ b/src/os/FDCache.h
@@ -86,7 +86,10 @@ public:
   void clear(const ghobject_t &hoid) {
     int registry_id = hoid.hobj.hash % registry_shards;
     registry[registry_id].clear(hoid);
-    assert(!registry[registry_id].lookup(hoid));
+    if (registry[registry_id].lookup(hoid)) {
+      registry[registry_id].dump_weak_refs();
+      assert(!registry[registry_id].lookup(hoid));
+    }
   }
 
   /// md_config_obs_t

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index e91bda69d37178f73d80518512499e64e45eb456..ab4e20581e534d8d8046d6f89037f28c1086394c 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -462,11 +462,16 @@ int FileStore::lfn_unlink(coll_t cid, const ghobject_t& o,
       r = ::stat(path->path(), &st);
       if (r < 0) {
        r = -errno;
-       assert(!m_filestore_fail_eio || r != -EIO);
+       if (r == -ENOENT) {
+         wbthrottle.clear_object(o); // should be only non-cache ref
+         fdcache.clear(o);
+       } else {
+         assert(!m_filestore_fail_eio || r != -EIO);
+       }
        return r;
-      }
-      if (st.st_nlink == 1)
+      } else if (st.st_nlink == 1) {
        force_clear_omap = true;
+      }
     }
     if (force_clear_omap) {
       dout(20) << __func__ << ": clearing omap on " << o

diff --git a/src/test/osd/osd-scrub-repair.sh b/src/test/osd/osd-scrub-repair.sh
new file mode 100755 (executable)
index 0000000..0c805b0
--- /dev/null
+++ b/src/test/osd/osd-scrub-repair.sh
@@ -0,0 +1,95 @@
+#!/bin/bash
+#
+# Copyright (C) 2014 Red Hat <contact@redhat.com>
+#
+# Author: Loic Dachary <loic@dachary.org>
+#
+# 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 test/mon/mon-test-helpers.sh
+source test/osd/osd-test-helpers.sh
+
+function run() {
+    local dir=$1
+
+    export CEPH_ARGS
+    CEPH_ARGS+="--fsid=$(uuidgen) --auth-supported=none "
+    CEPH_ARGS+="--mon-host=127.0.0.1 "
+
+    setup $dir || return 1
+    run_mon $dir a --public-addr 127.0.0.1 || return 1
+    for id in $(seq 0 3) ; do
+        run_osd $dir $id || return 1
+    done
+    FUNCTIONS=${FUNCTIONS:-$(set | sed -n -e 's/^\(TEST_[0-9a-z_]*\) .*/\1/p')}
+    for TEST_function in $FUNCTIONS ; do
+        if ! $TEST_function $dir ; then
+            cat $dir/a/log
+            return 1
+        fi
+    done
+    teardown $dir || return 1
+}
+
+# 
+# 1) add an object 
+# 2) remove the corresponding file from the primary OSD
+# 3) repair the PG
+#
+# Reproduces http://tracker.ceph.com/issues/8914
+#
+function TEST_bug_8914() {
+    local dir=$1
+    local poolname=rbd
+    local payload=ABCDEF
+
+    ./ceph osd set noscrub || return 1
+    ./ceph osd set nodeep-scrub || return 1
+
+    echo $payload > $dir/ORIGINAL
+    #
+    # 1) add an object 
+    #
+    ./rados --pool $poolname put SOMETHING $dir/ORIGINAL || return 1
+    local -a osds=($(get_osds $poolname SOMETHING))
+    local file=$(find $dir/${osds[$first]} -name '*SOMETHING*')
+    local -i tries=0
+    while [ ! -f $file -a $tries -lt 100 ] ; do
+        let tries++
+        sleep 1
+    done
+    grep --quiet --recursive --text $payload $file || return 1
+    #
+    # 2) remove the corresponding file from the primary OSD
+    #
+    rm $file
+    #
+    # 3) repair the PG
+    #
+    local pg=$(get_pg $poolname SOMETHING)
+    ./ceph pg repair $pg
+    local -i tries=0
+    while [ ! -f $file -a $tries -lt 100 ] ; do
+        let tries++
+        sleep 1
+    done
+    #
+    # The file must be back
+    #
+    test -f $file || return 1
+}
+
+main osd-scrub-repair
+
+# Local Variables:
+# compile-command: "cd ../.. ; make -j4 && test/osd/osd-scrub-repair.sh"
+# End: