[chord] Problem with dhashcli

[Emil Sit]

On Sun, 10 February 2008 at 22:26 (-0500), Emil Sit wrote:
> regularly.  Can you try this patch to see if it fixes the problem?

Having slept on it, I think the updated patch in this mail should work
just as well, in addition to be simpler.  So, try this one instead,
if you can.  Thanks!

# HG changeset patch
# User Emil Sit <sit at mit.edu>
# Date 1202748843 18000
# Node ID c2c4016b96f02ee066dbdad678e547f3fc1807a3
# Parent  83fb3e1279d84d7a7fa5f8dee05b6e9bece04518
Fix update writes to mutable objects.

Since expiration removes keys from the Merkle tree, even though
they may still be on disk, dbns::del would return ADB_NOTFOUND
erroneously to dhblock_{noauth,keyhash}_srv's delete calls during
an update.  This caused an assert failure.  Since deletes failed,
adbd would then fail to process updates in dbns::insert because of
duplicate checking code (as seen in change c37026ba75f3).

The correct approach is to check for the object in the metadatadb
first, not in the Merkle tree.  This will allow both the delete and
the subsequent insert to succeed.  It might be worth developing an
atomic dbns::update function.

Found, in part, by Katarzyna Stefanowicz <kate_stefik at tlen.pl>

diff -r 83fb3e1279d8 -r c2c4016b96f0 lsd/adbd.C
--- a/lsd/adbd.C	Thu Jan 31 22:37:14 2008 -0500
+++ b/lsd/adbd.C	Mon Feb 11 11:54:03 2008 -0500
@@ -559,28 +559,31 @@ dbns::del (const chordID &key, u_int32_t
     return r;
   }
 
-  char *err = "";
-  do {
-    if (hasaux ()) {
-      err = "mtree->remove aux";
-      r = mtree->remove (key, auxdata, t);
-      if (r) break;
-    } else {
-      err = "mtree->remove";
-      r = mtree->remove (key, t);
-      if (r) break;
-    }
-    err = "metadatadb->del";
-    r = metadatadb->del (metadatadb, t, &skey, 0);
-    if (r) break;
-  } while (0);
-
-  int ret = 0;
+  r = metadatadb->del (metadatadb, t, &skey, 0);
   if (r) {
     if (r != DB_NOTFOUND)
-      warner ("dbns::remove", err, r);
+      warner ("dbns::remove", "metadatadb->del", r);
+    dbfe_txn_abort (dbe, t);
+    return r;
+  }
+
+  // Only attempt to update Merkle tree if object was present.
+  char *err = "";
+  if (hasaux ()) {
+    err = "mtree->remove aux";
+    r = mtree->remove (key, auxdata, t);
+  } else {
+    err = "mtree->remove";
+    r = mtree->remove (key, t);
+  }
+  int ret = 0;
+  if (r && r != DB_NOTFOUND) {
+    warner ("dbns::remove", err, r);
     ret = dbfe_txn_abort (dbe, t);
   } else {
+    // Ignore any NOTFOUND errors since the Merkle
+    // key may have been removed by expiration.
+    r = 0;
     ret = dbfe_txn_commit (dbe, t);
   }
   if (ret)




-- 
Emil Sit / MIT CSAIL PDOS / http://pdos.csail.mit.edu/chord/