]> git.ozlabs.org Git - ccan/blobdiff - ccan/tdb/tools/replay_trace.c
Handle transactions!
[ccan] / ccan / tdb / tools / replay_trace.c
index d58125b355e0ebe1a3c5f959bbe5aed95c0563f7..e639dcdc6ce656e132230c2057003431daa76623 100644 (file)
@@ -22,7 +22,7 @@
 /* Avoid mod by zero */
 static unsigned int total_keys = 1;
 
-/* #define DEBUG_DEPS 1 */
+#define DEBUG_DEPS 1
 
 /* Traversals block transactions in the current implementation. */
 #define TRAVERSALS_TAKE_TRANSACTION_LOCK 1
@@ -310,30 +310,36 @@ static void op_add_transaction(const char *filename, struct op op[],
        op[op_num].group_len = 0;
 }
 
+static int op_transaction_start(struct op op[], unsigned int op_num)
+{
+       unsigned int i;
+
+       for (i = op_num-1; i > 0; i--) {
+               if (op[i].op == OP_TDB_TRANSACTION_START && !op[i].group_len)
+                       return i;
+       }
+       return 0;
+}
+
 static void op_analyze_transaction(const char *filename,
                                   struct op op[], unsigned int op_num,
                                   char *words[])
 {
-       int i, start;
+       unsigned int start, i;
 
        op[op_num].key = tdb_null;
 
        if (words[2])
                fail(filename, op_num+1, "Expect no arguments");
 
-       for (i = op_num-1; i >= 0; i--) {
-               if (op[i].op == OP_TDB_TRANSACTION_START && !op[i].group_len)
-                       break;
-       }
-
-       if (i < 0)
+       start = op_transaction_start(op, op_num);
+       if (!start)
                fail(filename, op_num+1, "no transaction start found");
 
-       start = i;
-       op[start].group_len = op_num - i;
+       op[start].group_len = op_num - start;
 
        /* This rolls in nested transactions.  I think that's right. */
-       for (i++; i <= op_num; i++)
+       for (i = start; i <= op_num; i++)
                op[i].group_start = start;
 }
 
@@ -404,23 +410,26 @@ static void check_deps(const char *filename, struct op op[], unsigned int num)
 #endif
 }
 
-static void dump_pre(char *filename[], unsigned int file,
-                    struct op op[], unsigned int i)
+static void dump_pre(char *filename[], struct op *op[],
+                    unsigned int file, unsigned int i)
 {
        struct depend *dep;
 
-       printf("%s:%u still waiting for:\n", filename[file], i+1);
-       list_for_each(&op[i].pre, dep, pre_list)
-               printf("    %s:%u\n",
-                      filename[dep->satisfies_file], dep->satisfies_opnum+1);
-       check_deps(filename[file], op, i);
+       printf("%s:%u (%u) still waiting for:\n", filename[file], i+1,
+               op[file][i].serial);
+       list_for_each(&op[file][i].pre, dep, pre_list)
+               printf("    %s:%u (%u)\n",
+                      filename[dep->satisfies_file], dep->satisfies_opnum+1,
+                      op[dep->satisfies_file][dep->satisfies_opnum].serial);
+       check_deps(filename[file], op[file], i);
 }
 
 /* We simply read/write pointers, since we all are children. */
-static void do_pre(char *filename[], unsigned int file, int pre_fd,
-                  struct op op[], unsigned int i)
+static void do_pre(struct tdb_context *tdb,
+                  char *filename[], struct op *op[],
+                  unsigned int file, int pre_fd, unsigned int i)
 {
-       while (!list_empty(&op[i].pre)) {
+       while (!list_empty(&op[file][i].pre)) {
                struct depend *dep;
 
 #if DEBUG_DEPS
@@ -430,7 +439,7 @@ static void do_pre(char *filename[], unsigned int file, int pre_fd,
                alarm(10);
                while (read(pre_fd, &dep, sizeof(dep)) != sizeof(dep)) {
                        if (errno == EINTR) {
-                               dump_pre(filename, file, op, i);
+                               dump_pre(filename, op, file, i);
                                exit(1);
                        } else
                                errx(1, "Reading from pipe");
@@ -448,12 +457,12 @@ static void do_pre(char *filename[], unsigned int file, int pre_fd,
        }
 }
 
-static void do_post(char *filename[], unsigned int file,
-                   const struct op op[], unsigned int i)
+static void do_post(char *filename[], struct op *op[],
+                   unsigned int file, unsigned int i)
 {
        struct depend *dep;
 
-       list_for_each(&op[i].post, dep, post_list) {
+       list_for_each(&op[file][i].post, dep, post_list) {
 #if DEBUG_DEPS
                printf("%s:%u:sending to file %s:%u\n", filename[file], i+1,
                       filename[dep->needs_file], dep->needs_opnum+1);
@@ -473,12 +482,12 @@ static int get_len(TDB_DATA key, TDB_DATA data, void *private_data)
 static unsigned run_ops(struct tdb_context *tdb,
                        int pre_fd,
                        char *filename[],
+                       struct op *op[],
                        unsigned int file,
-                       struct op op[],
                        unsigned int start, unsigned int stop);
 
 struct traverse_info {
-       struct op *op;
+       struct op **op;
        char **filename;
        unsigned file;
        int pre_fd;
@@ -492,7 +501,7 @@ static int nontrivial_traverse(struct tdb_context *tdb,
                               void *_tinfo)
 {
        struct traverse_info *tinfo = _tinfo;
-       unsigned int trav_len = tinfo->op[tinfo->start].group_len;
+       unsigned int trav_len = tinfo->op[tinfo->file][tinfo->start].group_len;
 
        if (tinfo->i == tinfo->start + trav_len) {
                /* This can happen if traverse expects to be empty. */
@@ -502,13 +511,13 @@ static int nontrivial_traverse(struct tdb_context *tdb,
                     "traverse did not terminate");
        }
 
-       if (tinfo->op[tinfo->i].op != OP_TDB_TRAVERSE)
+       if (tinfo->op[tinfo->file][tinfo->i].op != OP_TDB_TRAVERSE)
                fail(tinfo->filename[tinfo->file], tinfo->start + 1,
                     "%s:%u:traverse terminated early");
 
        /* Run any normal ops. */
-       tinfo->i = run_ops(tdb, tinfo->pre_fd, tinfo->filename, tinfo->file,
-                          tinfo->op, tinfo->i+1, tinfo->start + trav_len);
+       tinfo->i = run_ops(tdb, tinfo->pre_fd, tinfo->filename, tinfo->op,
+                          tinfo->file, tinfo->i+1, tinfo->start + trav_len);
 
        if (tinfo->i == tinfo->start + trav_len)
                return 1;
@@ -522,7 +531,7 @@ static unsigned op_traverse(struct tdb_context *tdb,
                            unsigned int file,
                            int (*traversefn)(struct tdb_context *,
                                              tdb_traverse_func, void *),
-                           struct op op[],
+                           struct op *op[],
                            unsigned int start)
 {
        struct traverse_info tinfo = { op, filename, file, pre_fd,
@@ -533,12 +542,13 @@ static unsigned op_traverse(struct tdb_context *tdb,
        /* Traversing in wrong order can have strange effects: eg. if
         * original traverse went A (delete A), B, we might do B
         * (delete A).  So if we have ops left over, we do it now. */
-       while (tinfo.i != start + op[start].group_len) {
-               if (op[tinfo.i].op == OP_TDB_TRAVERSE)
+       while (tinfo.i != start + op[file][start].group_len) {
+               if (op[file][tinfo.i].op == OP_TDB_TRAVERSE)
                        tinfo.i++;
                else
-                       tinfo.i = run_ops(tdb, pre_fd, filename, file, op,
-                                         tinfo.i, start + op[start].group_len);
+                       tinfo.i = run_ops(tdb, pre_fd, filename, op, file,
+                                         tinfo.i,
+                                         start + op[file][start].group_len);
        }
 
        return tinfo.i;
@@ -552,8 +562,9 @@ static __attribute__((noinline))
 unsigned run_ops(struct tdb_context *tdb,
                 int pre_fd,
                 char *filename[],
+                struct op *op[],
                 unsigned int file,
-                struct op op[], unsigned int start, unsigned int stop)
+                unsigned int start, unsigned int stop)
 {
        unsigned int i;
        struct sigaction sa;
@@ -563,88 +574,98 @@ unsigned run_ops(struct tdb_context *tdb,
 
        sigaction(SIGALRM, &sa, NULL);
        for (i = start; i < stop; i++) {
-               do_pre(filename, file, pre_fd, op, i);
+               do_pre(tdb, filename, op, file, pre_fd, i);
 
-               switch (op[i].op) {
+               switch (op[file][i].op) {
                case OP_TDB_LOCKALL:
-                       try(tdb_lockall(tdb), op[i].ret);
+                       try(tdb_lockall(tdb), op[file][i].ret);
                        break;
                case OP_TDB_LOCKALL_MARK:
-                       try(tdb_lockall_mark(tdb), op[i].ret);
+                       try(tdb_lockall_mark(tdb), op[file][i].ret);
                        break;
                case OP_TDB_LOCKALL_UNMARK:
-                       try(tdb_lockall_unmark(tdb), op[i].ret);
+                       try(tdb_lockall_unmark(tdb), op[file][i].ret);
                        break;
                case OP_TDB_LOCKALL_NONBLOCK:
-                       unreliable(tdb_lockall_nonblock(tdb), op[i].ret,
+                       unreliable(tdb_lockall_nonblock(tdb), op[file][i].ret,
                                   tdb_lockall(tdb), tdb_unlockall(tdb));
                        break;
                case OP_TDB_UNLOCKALL:
-                       try(tdb_unlockall(tdb), op[i].ret);
+                       try(tdb_unlockall(tdb), op[file][i].ret);
                        break;
                case OP_TDB_LOCKALL_READ:
-                       try(tdb_lockall_read(tdb), op[i].ret);
+                       try(tdb_lockall_read(tdb), op[file][i].ret);
                        break;
                case OP_TDB_LOCKALL_READ_NONBLOCK:
-                       unreliable(tdb_lockall_read_nonblock(tdb), op[i].ret,
+                       unreliable(tdb_lockall_read_nonblock(tdb),
+                                  op[file][i].ret,
                                   tdb_lockall_read(tdb),
                                   tdb_unlockall_read(tdb));
                        break;
                case OP_TDB_UNLOCKALL_READ:
-                       try(tdb_unlockall_read(tdb), op[i].ret);
+                       try(tdb_unlockall_read(tdb), op[file][i].ret);
                        break;
                case OP_TDB_CHAINLOCK:
-                       try(tdb_chainlock(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainlock(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_CHAINLOCK_NONBLOCK:
-                       unreliable(tdb_chainlock_nonblock(tdb, op[i].key),
-                                  op[i].ret,
-                                  tdb_chainlock(tdb, op[i].key),
-                                  tdb_chainunlock(tdb, op[i].key));
+                       unreliable(tdb_chainlock_nonblock(tdb, op[file][i].key),
+                                  op[file][i].ret,
+                                  tdb_chainlock(tdb, op[file][i].key),
+                                  tdb_chainunlock(tdb, op[file][i].key));
                        break;
                case OP_TDB_CHAINLOCK_MARK:
-                       try(tdb_chainlock_mark(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainlock_mark(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_CHAINLOCK_UNMARK:
-                       try(tdb_chainlock_unmark(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainlock_unmark(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_CHAINUNLOCK:
-                       try(tdb_chainunlock(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainunlock(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_CHAINLOCK_READ:
-                       try(tdb_chainlock_read(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainlock_read(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_CHAINUNLOCK_READ:
-                       try(tdb_chainunlock_read(tdb, op[i].key), op[i].ret);
+                       try(tdb_chainunlock_read(tdb, op[file][i].key),
+                           op[file][i].ret);
                        break;
                case OP_TDB_PARSE_RECORD:
-                       try(tdb_parse_record(tdb, op[i].key, get_len, NULL),
-                           op[i].ret);
+                       try(tdb_parse_record(tdb, op[file][i].key, get_len,
+                                            NULL),
+                           op[file][i].ret);
                        break;
                case OP_TDB_EXISTS:
-                       try(tdb_exists(tdb, op[i].key), op[i].ret);
+                       try(tdb_exists(tdb, op[file][i].key), op[file][i].ret);
                        break;
                case OP_TDB_STORE:
-                       try(tdb_store(tdb, op[i].key, op[i].data, op[i].flag),
-                           op[i].ret);
+                       try(tdb_store(tdb, op[file][i].key, op[file][i].data,
+                                     op[file][i].flag),
+                           op[file][i].ret);
                        break;
                case OP_TDB_APPEND:
-                       try(tdb_append(tdb, op[i].key, op[i].data), op[i].ret);
+                       try(tdb_append(tdb, op[file][i].key, op[file][i].data),
+                           op[file][i].ret);
                        break;
                case OP_TDB_GET_SEQNUM:
-                       try(tdb_get_seqnum(tdb), op[i].ret);
+                       try(tdb_get_seqnum(tdb), op[file][i].ret);
                        break;
                case OP_TDB_WIPE_ALL:
-                       try(tdb_wipe_all(tdb), op[i].ret);
+                       try(tdb_wipe_all(tdb), op[file][i].ret);
                        break;
                case OP_TDB_TRANSACTION_START:
-                       try(tdb_transaction_start(tdb), op[i].ret);
+                       try(tdb_transaction_start(tdb), op[file][i].ret);
                        break;
                case OP_TDB_TRANSACTION_CANCEL:
-                       try(tdb_transaction_cancel(tdb), op[i].ret);
+                       try(tdb_transaction_cancel(tdb), op[file][i].ret);
                        break;
                case OP_TDB_TRANSACTION_COMMIT:
-                       try(tdb_transaction_commit(tdb), op[i].ret);
+                       try(tdb_transaction_commit(tdb), op[file][i].ret);
                        break;
                case OP_TDB_TRAVERSE_READ_START:
                        i = op_traverse(tdb, pre_fd, filename, file,
@@ -662,29 +683,47 @@ unsigned run_ops(struct tdb_context *tdb,
                        fail(filename[file], i+1, "unexpected end traverse");
                /* FIXME: These must be treated like traverse. */
                case OP_TDB_FIRSTKEY:
-                       if (!key_eq(tdb_firstkey(tdb), op[i].data))
+                       if (!key_eq(tdb_firstkey(tdb), op[file][i].data))
                                fail(filename[file], i+1, "bad firstkey");
                        break;
                case OP_TDB_NEXTKEY:
-                       if (!key_eq(tdb_nextkey(tdb, op[i].key), op[i].data))
+                       if (!key_eq(tdb_nextkey(tdb, op[file][i].key),
+                                   op[file][i].data))
                                fail(filename[file], i+1, "bad nextkey");
                        break;
                case OP_TDB_FETCH: {
-                       TDB_DATA f = tdb_fetch(tdb, op[i].key);
-                       if (!key_eq(f, op[i].data))
+                       TDB_DATA f = tdb_fetch(tdb, op[file][i].key);
+                       if (!key_eq(f, op[file][i].data))
                                fail(filename[file], i+1, "bad fetch %u",
                                     f.dsize);
                        break;
                }
                case OP_TDB_DELETE:
-                       try(tdb_delete(tdb, op[i].key), op[i].ret);
+                       try(tdb_delete(tdb, op[file][i].key), op[file][i].ret);
                        break;
                }
-               do_post(filename, file, op, i);
+               do_post(filename, op, file, i);
        }
        return i;
 }
 
+/* tdbtorture, in particular, can do a tdb_close with a transaction in
+ * progress. */
+static struct op *maybe_cancel_transaction(const char *filename,
+                                          struct op *op, unsigned int *num)
+{
+       unsigned int start = op_transaction_start(op, *num);
+
+       if (start) {
+               char *words[] = { "<unknown>", "tdb_close", NULL };
+               add_op(filename, &op, *num, op[start].serial,
+                      OP_TDB_TRANSACTION_CANCEL);
+               op_analyze_transaction(filename, op, *num, words);
+               (*num)++;
+       }
+       return op;
+}
+
 static struct op *load_tracefile(const char *filename, unsigned int *num,
                                 unsigned int *hashsize,
                                 unsigned int *tdb_flags,
@@ -727,7 +766,8 @@ static struct op *load_tracefile(const char *filename, unsigned int *num,
                                             "lines after tdb_close");
                                *num = i;
                                talloc_free(lines);
-                               return op;
+                               return maybe_cancel_transaction(filename,
+                                                               op, num);
                        }
                        fail(filename, i+1, "Unknown operation '%s'", words[1]);
                }
@@ -740,7 +780,7 @@ static struct op *load_tracefile(const char *filename, unsigned int *num,
              filename, i);
        talloc_free(lines);
        *num = i - 1;
-       return op;
+       return maybe_cancel_transaction(filename, op, num);
 }
 
 /* We remember all the keys we've ever seen, and who has them. */
@@ -833,9 +873,32 @@ static const TDB_DATA *needs(const struct op *op)
        
 }
 
+static bool is_transaction(const struct op *op)
+{
+       return op->op == OP_TDB_TRANSACTION_START;
+}
+
 /* What's the data after this op?  pre if nothing changed. */
-static const TDB_DATA *gives(const struct op *op, const TDB_DATA *pre)
+static const TDB_DATA *gives(const TDB_DATA *key, const TDB_DATA *pre,
+                            const struct op *op)
 {
+       if (is_transaction(op)) {
+               unsigned int i;
+
+               /* Cancelled transactions don't change anything. */
+               if (op[op->group_len].op == OP_TDB_TRANSACTION_CANCEL)
+                       return pre;
+               assert(op[op->group_len].op == OP_TDB_TRANSACTION_COMMIT);
+
+               for (i = 1; i < op->group_len; i++) {
+                       /* This skips nested transactions, too */
+                       if (op[i].op != OP_TDB_TRAVERSE
+                           && key_eq(op[i].key, *key))
+                               pre = gives(key, pre, &op[i]);
+               }
+               return pre;
+       }
+
        /* Failed ops don't change state of db. */
        if (op->ret < 0)
                return pre;
@@ -852,6 +915,16 @@ static const TDB_DATA *gives(const struct op *op, const TDB_DATA *pre)
        return pre;
 }
 
+static bool in_transaction(const struct op op[], unsigned int i)
+{
+       return op[i].group_start && is_transaction(&op[op[i].group_start]);
+}
+
+static bool in_traverse(const struct op op[], unsigned int i)
+{
+       return op[i].group_start && !is_transaction(&op[op[i].group_start]);
+}
+
 static struct keyinfo *hash_ops(struct op *op[], unsigned int num_ops[],
                                unsigned int num)
 {
@@ -889,7 +962,23 @@ static struct keyinfo *hash_ops(struct op *op[], unsigned int num_ops[],
                        hash[h].user = talloc_realloc(hash, hash[h].user,
                                                     struct key_user,
                                                     hash[h].num_users+1);
-                       hash[h].user[hash[h].num_users].op_num = j;
+
+                       /* If it's in a transaction, it's the transaction which
+                        * matters from an analysis POV. */
+                       if (in_transaction(op[i], j)) {
+                               unsigned start = op[i][j].group_start;
+
+                               /* Don't include twice. */
+                               if (hash[h].num_users
+                                   && hash[h].user[hash[h].num_users-1].file
+                                       == i
+                                   && hash[h].user[hash[h].num_users-1].op_num
+                                       == start)
+                                       continue;
+
+                               hash[h].user[hash[h].num_users].op_num = start;
+                       } else
+                               hash[h].user[hash[h].num_users].op_num = j;
                        hash[h].user[hash[h].num_users].file = i;
                        hash[h].num_users++;
                }
@@ -898,8 +987,33 @@ static struct keyinfo *hash_ops(struct op *op[], unsigned int num_ops[],
        return hash;
 }
 
-static bool satisfies(const TDB_DATA *data, const TDB_DATA *need)
+static bool satisfies(const TDB_DATA *key, const TDB_DATA *data,
+                     const struct op *op)
 {
+       const TDB_DATA *need = NULL;
+
+       if (is_transaction(op)) {
+               unsigned int i;
+
+               /* Look through for an op in this transaction which
+                * needs this key. */
+               for (i = 1; i < op->group_len; i++) {
+                       if (op[i].op != OP_TDB_TRAVERSE
+                           && key_eq(op[i].key, *key)) {
+                               need = needs(&op[i]);
+                               /* tdb_exists() is special: there might be
+                                * something in the transaction with more
+                                * specific requirements.  Other ops don't have
+                                * specific requirements (eg. store or delete),
+                                * but they change the value so we can't get
+                                * more information from future ops. */
+                               if (op[i].op != OP_TDB_EXISTS)
+                                       break;
+                       }
+               }
+       } else
+               need = needs(op);
+
        /* Don't need anything?  Cool. */
        if (!need)
                return true;
@@ -930,34 +1044,50 @@ static bool satisfies(const TDB_DATA *data, const TDB_DATA *need)
        return key_eq(*data, *need);
 }
 
-static void move_to_front(struct key_user res[], unsigned int elem)
+static void move_to_front(struct key_user res[], unsigned off, unsigned elem)
 {
-       if (elem != 0) {
+       if (elem != off) {
                struct key_user tmp = res[elem];
-               memmove(res + 1, res, elem*sizeof(res[0]));
-               res[0] = tmp;
+               memmove(res + off + 1, res + off, (elem - off)*sizeof(res[0]));
+               res[off] = tmp;
        }
 }
 
-static void restore_to_pos(struct key_user res[], unsigned int elem)
+static void restore_to_pos(struct key_user res[], unsigned off, unsigned elem)
 {
-       if (elem != 0) {
-               struct key_user tmp = res[0];
-               memmove(res, res + 1, elem*sizeof(res[0]));
+       if (elem != off) {
+               struct key_user tmp = res[off];
+               memmove(res + off, res + off + 1, (elem - off)*sizeof(res[0]));
                res[elem] = tmp;
        }
 }
 
 static bool sort_deps(char *filename[], struct op *op[],
-                     struct key_user res[], unsigned num,
-                     const TDB_DATA *data, unsigned num_files)
+                     struct key_user res[],
+                     unsigned off, unsigned num,
+                     const TDB_DATA *key, const TDB_DATA *data,
+                     unsigned num_files, unsigned fuzz)
 {
        unsigned int i, files_done;
        struct op *this_op;
        bool done[num_files];
 
-       /* Nothing left?  We're sorted. */
-       if (num == 0)
+       /* Does this make serial numbers go backwards?  Allow a little fuzz. */
+       if (off > 0) {
+               int serial1 = op[res[off-1].file][res[off-1].op_num].serial;
+               int serial2 = op[res[off].file][res[off].op_num].serial;
+
+               if (serial1 - serial2 > (int)fuzz) {
+#if DEBUG_DEPS
+                       printf("Serial jump too far (%u -> %u)\n",
+                              serial1, serial2);
+#endif
+                       return false;
+               }
+       }
+
+       /* One or none left?  We're sorted. */
+       if (off + 1 >= num)
                return true;
 
        memset(done, 0, sizeof(done));
@@ -966,18 +1096,20 @@ static bool sort_deps(char *filename[], struct op *op[],
         * out which file to try next.  Since we don't take into account
         * inter-key relationships (which exist by virtue of trace file order),
         * we minimize the chance of harm by trying to keep in serial order. */
-       for (files_done = 0, i = 0; i < num && files_done < num_files; i++) {
+       for (files_done = 0, i = off; i < num && files_done < num_files; i++) {
                if (done[res[i].file])
                        continue;
 
                this_op = &op[res[i].file][res[i].op_num];
+
                /* Is what we have good enough for this op? */
-               if (satisfies(data, needs(this_op))) {
-                       move_to_front(res, i);
-                       if (sort_deps(filename, op, res+1, num-1,
-                                     gives(this_op, data), num_files))
+               if (satisfies(key, data, this_op)) {
+                       move_to_front(res, off, i);
+                       if (sort_deps(filename, op, res, off+1, num,
+                                     key, gives(key, data, this_op),
+                                     num_files, fuzz))
                                return true;
-                       restore_to_pos(res, i);
+                       restore_to_pos(res, off, i);
                }
                done[res[i].file] = true;
                files_done++;
@@ -1002,7 +1134,7 @@ static void check_dep_sorting(struct key_user user[], unsigned num_users,
 #endif
 }
 
-/* All these ops have the same serial number.  Which comes first?
+/* All these ops happen on the same key.  Which comes first?
  *
  * This can happen both because read ops or failed write ops don't
  * change serial number, and also due to race since we access the
@@ -1010,13 +1142,22 @@ static void check_dep_sorting(struct key_user user[], unsigned num_users,
  * in which case we'll deadlock and report: fix manually in that case).
  */
 static void figure_deps(char *filename[], struct op *op[],
-                       struct key_user user[], unsigned num_users,
-                       unsigned num_files)
+                       const TDB_DATA *key, struct key_user user[],
+                       unsigned num_users, unsigned num_files)
 {
        /* We assume database starts empty. */
        const struct TDB_DATA *data = &tdb_null;
+       unsigned int fuzz;
 
-       if (!sort_deps(filename, op, user, num_users, data, num_files))
+       /* We prefer to keep strict serial order if possible: it's the
+        * most likely.  We get more lax if that fails. */
+       for (fuzz = 0; fuzz < 100; fuzz = (fuzz + 1)*2) {
+               if (sort_deps(filename, op, user, 0, num_users, key, data,
+                             num_files, fuzz))
+                       break;
+       }
+
+       if (fuzz >= 100)
                fail(filename[user[0].file], user[0].op_num+1,
                     "Could not resolve inter-dependencies");
 
@@ -1047,7 +1188,8 @@ static void sort_ops(struct keyinfo hash[], char *filename[], struct op *op[],
                struct key_user *user = hash[h].user;
 
                qsort(user, hash[h].num_users, sizeof(user[0]), compare_serial);
-               figure_deps(filename, op, user, hash[h].num_users, num);
+               figure_deps(filename, op, &hash[h].key, user, hash[h].num_users,
+                           num);
        }
 }
 
@@ -1067,7 +1209,6 @@ static void add_dependency(void *ctx,
                           unsigned int satisfies_opnum)
 {
        struct depend *dep;
-       unsigned int needs_start, sat_start;
 
        /* We don't depend on ourselves. */
        if (needs_file == satisfies_file) {
@@ -1081,37 +1222,57 @@ static void add_dependency(void *ctx,
               filename[satisfies_file], satisfies_opnum+1);
 #endif
 
-       needs_start = op[needs_file][needs_opnum].group_start;
-       sat_start = op[satisfies_file][satisfies_opnum].group_start;
+#if TRAVERSALS_TAKE_TRANSACTION_LOCK
+       /* If something in a traverse depends on something in another
+        * traverse/transaction, it creates a dependency between the
+        * two groups. */
+       if ((in_traverse(op[satisfies_file], satisfies_opnum)
+            && op[needs_file][needs_opnum].group_start)
+           || (in_traverse(op[needs_file], needs_opnum)
+               && op[satisfies_file][satisfies_opnum].group_start)) {
+               unsigned int sat;
+
+               /* We are satisfied by end of group. */
+               sat = op[satisfies_file][satisfies_opnum].group_start;
+               satisfies_opnum = sat + op[satisfies_file][sat].group_len;
+               /* And we need that done by start of our group. */
+               needs_opnum = op[needs_file][needs_opnum].group_start;
+       }
 
-       /* If needs is in a transaction, we need it before start. */
-       if (needs_start) {
-               switch (op[needs_file][needs_start].op) {
-               case OP_TDB_TRANSACTION_START:
-                       needs_opnum = needs_start;
-#ifdef DEBUG_DEPS
-                       printf("  -> Back to %u\n", needs_start+1);
-                       fflush(stdout);
-#endif
-                       break;
-               default:
-                       break;
+       /* There is also this case:
+        *  <traverse> <read foo> ...
+        *  <transaction> ... </transaction> <create foo>
+        * Where if we start the traverse then wait, we could block
+        * the transaction and deadlock.
+        *
+        * We try to address this by ensuring that where seqnum indicates it's
+        * possible, we wait for <create foo> before *starting* traverse.
+        */
+       else if (in_traverse(op[needs_file], needs_opnum)) {
+               struct op *need = &op[needs_file][needs_opnum];
+               if (op[needs_file][need->group_start].serial <
+                   op[satisfies_file][satisfies_opnum].serial) {
+                       needs_opnum = need->group_start;
                }
        }
+#endif
 
-       /* If satisfies is in a transaction, we wait until after commit. */
-       /* FIXME: If transaction is cancelled, don't need dependency. */
-       if (sat_start) {
-               if (op[satisfies_file][sat_start].op
-                   == OP_TDB_TRANSACTION_START) {
-                       satisfies_opnum = sat_start
-                               + op[satisfies_file][sat_start].group_len;
-#ifdef DEBUG_DEPS
-                       printf("  -> Depends on %u\n", satisfies_opnum+1);
-                       fflush(stdout);
+       /* If you depend on a transaction, you actually depend on it ending. */
+       if (is_transaction(&op[satisfies_file][satisfies_opnum])) {
+               satisfies_opnum
+                       += op[satisfies_file][satisfies_opnum].group_len;
+#if DEBUG_DEPS
+               printf("-> Actually end of transaction %s:%u\n",
+                      filename[satisfies_file], satisfies_opnum+1);
 #endif
-               }
-       }
+       } else
+               /* We should never create a dependency from middle of
+                * a transaction. */
+               assert(!in_transaction(op[satisfies_file], satisfies_opnum)
+                      || op[satisfies_file][satisfies_opnum].op
+                      == OP_TDB_TRANSACTION_COMMIT
+                      || op[satisfies_file][satisfies_opnum].op
+                      == OP_TDB_TRANSACTION_CANCEL);
 
        assert(op[needs_file][needs_opnum].op != OP_TDB_TRAVERSE);
        assert(op[satisfies_file][satisfies_opnum].op != OP_TDB_TRAVERSE);
@@ -1126,65 +1287,9 @@ static void add_dependency(void *ctx,
        talloc_set_destructor(dep, destroy_depend);
 }
 
-#if TRAVERSALS_TAKE_TRANSACTION_LOCK
-struct traverse_dep {
-       unsigned int file;
-       unsigned int op_num;
-};
-
-/* Traversals can deadlock against each other.  Force order. */
-static void make_traverse_depends(char *filename[],
-                                 struct op *op[], unsigned int num_ops[],
-                                 unsigned int num)
+static bool changes_db(const TDB_DATA *key, const struct op *op)
 {
-       unsigned int i, j, num_traversals = 0;
-       struct traverse_dep *dep;
-
-       /* Sort by which one runs first. */
-       int compare_traverse_dep(const void *_a, const void *_b)
-       {
-               const struct traverse_dep *ta = _a, *tb = _b;
-               const struct op *a = &op[ta->file][ta->op_num],
-                       *b = &op[tb->file][tb->op_num];
-
-               if (a->serial != b->serial)
-                       return a->serial - b->serial;
-
-               /* If they have same serial, it means one didn't make any
-                * changes.  Thus sort by end in that case. */
-               return a[a->group_len].serial - b[b->group_len].serial;
-       }
-
-       dep = talloc_array(NULL, struct traverse_dep, 1);
-
-       /* Count them. */
-       for (i = 0; i < num; i++) {
-               for (j = 1; j < num_ops[i]; j++) {
-                       /* Transaction on traverse start. */
-                       if (op[i][j].group_start == j) {
-                               dep = talloc_realloc(NULL, dep,
-                                                    struct traverse_dep,
-                                                    num_traversals+1);
-                               dep[num_traversals].file = i;
-                               dep[num_traversals].op_num = j;
-                               num_traversals++;
-                       }
-               }
-       }
-       qsort(dep, num_traversals, sizeof(dep[0]), compare_traverse_dep);
-       for (i = 1; i < num_traversals; i++) {
-               /* i depends on end of traverse i-1. */
-               add_dependency(NULL, op, filename, dep[i].file, dep[i].op_num,
-                              dep[i-1].file, dep[i-1].op_num
-                              + op[dep[i-1].file][dep[i-1].op_num].group_len);
-       }
-       talloc_free(dep);
-}
-#endif /* TRAVERSALS_TAKE_TRANSACTION_LOCK */
-
-static bool changes_db(const struct op *op)
-{
-       return gives(op, NULL) != NULL;
+       return gives(key, NULL, op) != NULL;
 }
 
 static void depend_on_previous(struct op *op[],
@@ -1300,7 +1405,7 @@ static void derive_dependencies(char *filename[],
                        continue;
 
                for (i = 0; i < hash[h].num_users; i++) {
-                       if (changes_db(&op[hash[h].user[i].file]
+                       if (changes_db(&hash[h].key, &op[hash[h].user[i].file]
                                       [hash[h].user[i].op_num])) {
                                depend_on_previous(op, filename, num,
                                                   hash[h].user, i, prev);
@@ -1314,10 +1419,6 @@ static void derive_dependencies(char *filename[],
                }
        }
 
-#if TRAVERSALS_TAKE_TRANSACTION_LOCK
-       make_traverse_depends(filename, op, num_ops, num);
-#endif
-
        optimize_dependencies(op, num_ops, num);
 }
 
@@ -1358,7 +1459,7 @@ int main(int argc, char *argv[])
                printf("Single threaded run...");
                fflush(stdout);
 
-               run_ops(tdb, pipes[0].fd[0], argv+2, 0, op[0], 1, num_ops[0]);
+               run_ops(tdb, pipes[0].fd[0], argv+2, op, 0, 1, num_ops[0]);
                check_deps(argv[2], op[0], num_ops[0]);
 
                printf("done\n");
@@ -1385,7 +1486,7 @@ int main(int argc, char *argv[])
                        /* This catches parent exiting. */
                        if (read(fds[0], &c, 1) != 1)
                                exit(1);
-                       run_ops(tdb, pipes[i].fd[0], argv+2, i, op[i], 1,
+                       run_ops(tdb, pipes[i].fd[0], argv+2, op, i, 1,
                                num_ops[i]);
                        check_deps(argv[2+i], op[i], num_ops[i]);
                        exit(0);