PGLog: skip indexing errors by object

Dup detection only needs them indexed by version, and keeping them out
of the object index prevents error entries from contributing to the
missing set during recovery.

Signed-off-by: Josh Durgin <jdurgin@redhat.com>

diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 27c0f38c57653dca72ae8cbfc712a4b792f719e3..22d969f30254f3791852f751f47033ee84427650 100644 (file)
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -251,9 +251,10 @@ struct PGLog : DoutPrefixProvider {
       for (list<pg_log_entry_t>::iterator i = log.begin();
              i != log.end();
              ++i) {
-               
-        objects[i->soid] = &(*i);
-        
+       if (i->object_is_indexed()) {
+         objects[i->soid] = &(*i);
+       }
+
         if (i->reqid_is_indexed()) {
         //assert(caller_ops.count(i->reqid) == 0);  // divergent merge_log indexes new before unindexing old
           caller_ops[i->reqid] = &(*i);
@@ -277,7 +278,9 @@ struct PGLog : DoutPrefixProvider {
       for (list<pg_log_entry_t>::const_iterator i = log.begin();
             i != log.end();
             ++i) {
-         objects[i->soid] = const_cast<pg_log_entry_t*>(&(*i));
+       if (i->object_is_indexed()) {
+         objects[i->soid] = const_cast<pg_log_entry_t*>(&(*i));
+       }
        }
  
       indexed_data |= PGLOG_INDEXED_OBJECTS;
@@ -316,8 +319,8 @@ struct PGLog : DoutPrefixProvider {
     }
 
     void index(pg_log_entry_t& e) {
-      if (indexed_data & PGLOG_INDEXED_OBJECTS) {
-        if (objects.count(e.soid) == 0 || 
+      if ((indexed_data & PGLOG_INDEXED_OBJECTS) && e.object_is_indexed()) {
+        if (objects.count(e.soid) == 0 ||
             objects[e.soid]->version < e.version)
           objects[e.soid] = &e;
       }
@@ -393,7 +396,7 @@ struct PGLog : DoutPrefixProvider {
       head = e.version;
 
       // to our index
-      if (indexed_data & PGLOG_INDEXED_OBJECTS) {
+      if ((indexed_data & PGLOG_INDEXED_OBJECTS) && e.object_is_indexed()) {
         objects[e.soid] = &(log.back());
       }
       if (indexed_data & PGLOG_INDEXED_CALLER_OPS) {

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 67eb6840fecc317627da6facb1bc05894d8a863b..5145279c5938c125257eb454724e1e7d10821d01 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -2617,7 +2617,15 @@ struct pg_log_entry_t {
   bool is_delete() const {
     return op == DELETE || op == LOST_DELETE;
   }
-      
+
+  // Errors are only used for dup detection, whereas
+  // the index by objects is used by recovery, copy_get,
+  // and other facilities that don't expect or need to
+  // be aware of error entries.
+  bool object_is_indexed() const {
+    return !is_error();
+  }
+
   bool reqid_is_indexed() const {
     return reqid != osd_reqid_t() &&
       (op == MODIFY || op == DELETE || op == ERROR);

diff --git a/src/test/osd/TestPGLog.cc b/src/test/osd/TestPGLog.cc
index cc0738639c25748050c1702db73c71e56b5ad70d..f92cbe313004a480a53505f1c0f523ab8f295a39 100644 (file)
--- a/src/test/osd/TestPGLog.cc
+++ b/src/test/osd/TestPGLog.cc
@@ -2087,6 +2087,60 @@ TEST_F(PGLogTest, get_request) {
   }
 }
 
+TEST_F(PGLogTest, ErrorNotIndexedByObject) {
+  clear();
+
+  // make sure writes, deletes, and errors are found
+  hobject_t oid(object_t("objname"), "key", 123, 456, 0, "");
+  log.add(
+    pg_log_entry_t(pg_log_entry_t::ERROR, oid, eversion_t(6,2), eversion_t(3,4),
+                  1, osd_reqid_t(entity_name_t::CLIENT(777), 8, 1),
+                  utime_t(0,1), -ENOENT));
+
+  EXPECT_FALSE(log.logged_object(oid));
+
+  pg_log_entry_t modify(pg_log_entry_t::MODIFY, oid, eversion_t(6,3),
+                       eversion_t(3,4), 2,
+                       osd_reqid_t(entity_name_t::CLIENT(777), 8, 2),
+                       utime_t(1,2), 0);
+  log.add(modify);
+
+  EXPECT_TRUE(log.logged_object(oid));
+  pg_log_entry_t *entry = log.objects[oid];
+  EXPECT_EQ(modify.op, entry->op);
+  EXPECT_EQ(modify.version, entry->version);
+  EXPECT_EQ(modify.prior_version, entry->prior_version);
+  EXPECT_EQ(modify.user_version, entry->user_version);
+  EXPECT_EQ(modify.reqid, entry->reqid);
+
+  pg_log_entry_t del(pg_log_entry_t::DELETE, oid, eversion_t(7,4),
+                    eversion_t(7,4), 3,
+                    osd_reqid_t(entity_name_t::CLIENT(777), 8, 3),
+                    utime_t(10,2), 0);
+  log.add(del);
+
+  EXPECT_TRUE(log.logged_object(oid));
+  entry = log.objects[oid];
+  EXPECT_EQ(del.op, entry->op);
+  EXPECT_EQ(del.version, entry->version);
+  EXPECT_EQ(del.prior_version, entry->prior_version);
+  EXPECT_EQ(del.user_version, entry->user_version);
+  EXPECT_EQ(del.reqid, entry->reqid);
+
+  log.add(
+    pg_log_entry_t(pg_log_entry_t::ERROR, oid, eversion_t(7,5), eversion_t(7,4),
+                  3, osd_reqid_t(entity_name_t::CLIENT(777), 8, 4),
+                  utime_t(20,1), -ENOENT));
+
+  EXPECT_TRUE(log.logged_object(oid));
+  entry = log.objects[oid];
+  EXPECT_EQ(del.op, entry->op);
+  EXPECT_EQ(del.version, entry->version);
+  EXPECT_EQ(del.prior_version, entry->prior_version);
+  EXPECT_EQ(del.user_version, entry->user_version);
+  EXPECT_EQ(del.reqid, entry->reqid);
+}
+
 int main(int argc, char **argv) {
   vector<const char*> args;
   argv_to_vec(argc, (const char **)argv, args);

diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc
index e075592f516df32b77527f6dc2cba23586713204..ade7dd1a4411e12b55499df6595ccd7a4d7e836b 100644 (file)
--- a/src/test/osd/types.cc
+++ b/src/test/osd/types.cc
@@ -819,6 +819,8 @@ TEST(pg_missing_t, add_next_event)
     e.op = pg_log_entry_t::MODIFY;
     e.prior_version = eversion_t();
     EXPECT_TRUE(e.is_update());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_TRUE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_TRUE(missing.is_missing(oid));
@@ -842,6 +844,8 @@ TEST(pg_missing_t, add_next_event)
     e.op = pg_log_entry_t::CLONE;
     e.prior_version = eversion_t();
     EXPECT_TRUE(e.is_clone());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_FALSE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_TRUE(missing.is_missing(oid));
@@ -865,6 +869,8 @@ TEST(pg_missing_t, add_next_event)
     e.op = pg_log_entry_t::MODIFY;
     e.prior_version = eversion_t();
     EXPECT_TRUE(e.is_update());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_TRUE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_TRUE(missing.is_missing(oid));
@@ -889,6 +895,8 @@ TEST(pg_missing_t, add_next_event)
 
     e.op = pg_log_entry_t::MODIFY;
     EXPECT_TRUE(e.is_update());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_TRUE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_TRUE(missing.is_missing(oid));
@@ -906,6 +914,8 @@ TEST(pg_missing_t, add_next_event)
 
     e.op = pg_log_entry_t::BACKLOG;
     EXPECT_TRUE(e.is_backlog());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_FALSE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     EXPECT_DEATH(missing.add_next_event(e), "");
   }
@@ -917,6 +927,8 @@ TEST(pg_missing_t, add_next_event)
 
     e.op = pg_log_entry_t::MODIFY;
     EXPECT_TRUE(e.is_update());
+    EXPECT_TRUE(e.object_is_indexed());
+    EXPECT_TRUE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_TRUE(missing.is_missing(oid));
@@ -935,9 +947,12 @@ TEST(pg_missing_t, add_next_event)
     e.op = pg_log_entry_t::ERROR;
     e.return_code = -ENOENT;
     EXPECT_FALSE(e.is_update());
+    EXPECT_FALSE(e.object_is_indexed());
+    EXPECT_TRUE(e.reqid_is_indexed());
     EXPECT_FALSE(missing.is_missing(oid));
     missing.add_next_event(e);
     EXPECT_FALSE(missing.is_missing(oid));
+    EXPECT_FALSE(e.object_is_indexed());
     EXPECT_TRUE(e.reqid_is_indexed());
   }