I think you are missing a word in the first sentence of the commit message. Otherwise it looks good.
Ethan On Tue, Jun 14, 2011 at 15:01, Ben Pfaff <[email protected]> wrote: > 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 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
