Skip to content

Commit 1cc618c

Browse files
committed
ovsdb-idl: Fix atomicity of writes that don't change a column's value.
The existing ovsdb_idl_txn_commit() drops any writes that don't change a column's value from what was last reported by the database. But this isn't a valid optimization, because it breaks the atomicity of transactions. Suppose columns A and B initially have values 1 and 2. Client 1 writes value 1 to both columns in one transaction. Client 2 writes value 2 to both columns in another transaction. The only possible valid results for any serial ordering of transactions are 1,1 or 2,2. But if both clients drop writes to columns that they have not modified, then 2,1 also becomes possible (because client 1 just writes to B and client 2 just writes to A). However, for write-only columns we can optimize this out because the IDL can assume it is the only client writing to a column. Found by inspection.
1 parent 898ba40 commit 1cc618c

File tree

1 file changed

+16
-3
lines changed

1 file changed

+16
-3
lines changed

lib/ovsdb-idl.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,9 +1436,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
14361436
&class->columns[idx];
14371437

14381438
if (row->old
1439-
? !ovsdb_datum_equals(&row->old[idx], &row->new[idx],
1440-
&column->type)
1441-
: !ovsdb_datum_is_default(&row->new[idx],
1439+
|| !ovsdb_datum_is_default(&row->new[idx],
14421440
&column->type)) {
14431441
json_object_put(row_json, column->name,
14441442
substitute_uuids(
@@ -1633,6 +1631,21 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
16331631
assert(row->old == NULL ||
16341632
row->table->modes[column_idx] & OVSDB_IDL_MONITOR);
16351633

1634+
/* If this is a write-only column and the datum being written is the same
1635+
* as the one already there, just skip the update entirely. This is worth
1636+
* optimizing because we have a lot of columns that get periodically
1637+
* refreshed into the database but don't actually change that often.
1638+
*
1639+
* We don't do this for read/write columns because that would break
1640+
* atomicity of transactions--some other client might have written a
1641+
* different value in that column since we read it. */
1642+
if (row->table->modes[column_idx] == OVSDB_IDL_MONITOR
1643+
&& ovsdb_datum_equals(ovsdb_idl_read(row, column),
1644+
datum, &column->type)) {
1645+
ovsdb_datum_destroy(datum, &column->type);
1646+
return;
1647+
}
1648+
16361649
if (hmap_node_is_null(&row->txn_node)) {
16371650
hmap_insert(&row->table->idl->txn->txn_rows, &row->txn_node,
16381651
uuid_hash(&row->uuid));

0 commit comments

Comments
 (0)