ovsdb-idl: Optimize out transactions that are complete no-ops.
authorBen Pfaff <blp@nicira.com>
Tue, 14 Jun 2011 22:00:50 +0000 (15:00 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 15 Jun 2011 19:29:50 +0000 (12:29 -0700)
Commit 1cc618c3252 "ovsdb-idl: Fix atomicity of writes that don't change a
column's value" fixed transactions that write the existing value to
some columns, ensuring that those columns still got written to the database
to avoid making the transaction nonatomic in the presence of writes that do
modify part of the database.

However, that commit was too conservative: we can still optimize out a
database transaction that writes *only* existing values to the database,
because if we drop such a transaction then the resulting database is still
one that could result from executing transactions in a serial order.  This
commit implements that optimization.

As an example of what this commit does, before this commit, an "ovs-vsctl
set" command that specified the existing value for a column would do a
round-trip to the database to write that existing value.  After this
commit, that round-trip would not occur.

Found by observing system startup.

lib/ovsdb-idl.c

index fd15ea96f3f13103431ecdfeeda55720ca26e1d9..94784bdaabfb236e10351da9adbcac983dffa7a8 100644 (file)
@@ -1429,6 +1429,8 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
             } else {
                 struct ovsdb_idl_txn_insert *insert;
 
+                any_updates = true;
+
                 json_object_put(op, "uuid-name",
                                 json_string_create_nocopy(
                                     uuid_name_from_uuid(&row->uuid)));
@@ -1456,13 +1458,22 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                                             ovsdb_datum_to_json(&row->new[idx],
                                                                 &column->type),
                                             txn));
+
+                        /* If anything really changed, consider it an update.
+                         * We can't suppress not-really-changed values earlier
+                         * or transactions would become nonatomic (see the big
+                         * comment inside ovsdb_idl_txn_write()). */
+                        if (!any_updates && row->old &&
+                            !ovsdb_datum_equals(&row->old[idx], &row->new[idx],
+                                                &column->type)) {
+                            any_updates = true;
+                        }
                     }
                 }
             }
 
             if (!row->old || !shash_is_empty(json_object(row_json))) {
                 json_array_add(operations, op);
-                any_updates = true;
             } else {
                 json_destroy(op);
             }
@@ -1651,7 +1662,10 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
      *
      * We don't do this for read/write columns because that would break
      * atomicity of transactions--some other client might have written a
-     * different value in that column since we read it. */
+     * different value in that column since we read it.  (But if a whole
+     * transaction only does writes of existing values, without making any real
+     * changes, we will drop the whole transaction later in
+     * ovsdb_idl_txn_commit().) */
     if (row->table->modes[column_idx] == OVSDB_IDL_MONITOR
         && ovsdb_datum_equals(ovsdb_idl_read(row, column),
                               datum, &column->type)) {