[ovs-dev] [PATCH] ovsdb-idl: Optimize out transactions that are complete no-ops.

Ben Pfaff blp at nicira.com
Tue Jun 14 22:01:00 UTC 2011


Commit 1cc618c3252 "ovsdb-idl: Fix atomicity of writes that don't change a
column's value" the case of a transaction that wrote 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 |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index fd15ea9..94784bd 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -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)) {
-- 
1.7.4.4




More information about the dev mailing list