Thanks, I improved the commit message and will push soon.
On Tue, Jun 14, 2011 at 03:28:57PM -0700, Ethan Jackson wrote: > 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
