From c15f1d11fa95a036eaa2504527bbd5decf082db6 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 14 Jun 2011 15:00:50 -0700 Subject: [PATCH] ovsdb-idl: Optimize out transactions that are complete no-ops. 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 | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index fd15ea96..94784bda 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)) { -- 2.30.2