[ovs-dev] [PATCH 2/2] ovsdb-idl: Preserve references for deleted rows.
Dumitru Ceara
dceara at redhat.com
Tue Mar 2 12:44:58 UTC 2021
Considering two DB rows, 'a' from table A and 'b' from table B (with
column 'ref_a' a reference to table A):
a = {A._uuid=<U1>}
b = {B._uuid=<U2>, B.ref_a=<U1>}
Assuming both records are present in the IDL client's in-memory view of
the database, depending whether row 'b' is also deleted in the same
transaction or not, deletion of row 'a' should generate the following
tracked changes:
1. only row 'a' is deleted:
- for table A:
- deleted records: a = {A._uuid=<U1>}
- for table B:
- updated records: b = {B._uuid=<U2>, B.ref_a=[]}
2. row 'a' and row 'b' are deleted in the same update:
- for table A:
- deleted records: a = {A._uuid=<U1>}
- for table B:
- deleted records: b = {B._uuid=<U2>, B.ref_a=<U1>}
To ensure this, we now delay reparsing row backrefs until the row has
been removed from the table's hmap and until the IDL client has
processed all tracked changes (ovsdb_idl_track_clear() was called).
Without this change, in scenario 2 above, the tracked changes for table
B would be:
- deleted records: b = {B._uuid=<U2>, B.ref_a=[]}
In particular, for strong references, row 'a' can never be deleted in
a transaction that happens strictly before row 'b' is deleted. In some
cases [0] both rows are deleted in the same transaction and having
B.ref_a=[] would violate the integrity of the database from client
perspective. This would force the client to always validate that
strong reference fields are non-NULL. This is not really an option
because the information in the original reference is required for
incrementally processing the record deletion.
[0] with ovn-monitor-all=true, the following command triggers a crash
in ovn-controller because a strong reference field becomes NULL:
$ ovn-nbctl --wait=hv -- lr-add r -- lrp-add r rp 00:00:00:00:00:01 1.0.0.1/24
$ ovn-nbctl lr-del r
Reported-at: https://bugzilla.redhat.com/1932642
Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted rows.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
lib/ovsdb-idl.c | 6 ++--
tests/ovsdb-idl.at | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 2c8a0c9..38c6f28 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -163,6 +163,7 @@ static void ovsdb_idl_row_unparse(struct ovsdb_idl_row *);
static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *);
static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *);
static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, bool destroy_dsts);
+static void ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *row);
static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
static bool ovsdb_idl_txn_extract_mutations(struct ovsdb_idl_row *,
@@ -368,6 +369,8 @@ ovsdb_idl_clear(struct ovsdb_idl *db)
ovsdb_idl_row_unparse(row);
}
LIST_FOR_EACH_SAFE (arc, next_arc, src_node, &row->src_arcs) {
+ ovs_list_remove(&arc->src_node);
+ ovs_list_remove(&arc->dst_node);
free(arc);
}
/* No need to do anything with dst_arcs: some node has those arcs
@@ -1235,6 +1238,7 @@ ovsdb_idl_track_clear__(struct ovsdb_idl *idl, bool flush_all)
ovs_list_remove(&row->track_node);
ovs_list_init(&row->track_node);
if (ovsdb_idl_row_is_orphan(row)) {
+ ovsdb_idl_row_reparse_backrefs(row);
ovsdb_idl_row_unparse(row);
if (row->tracked_old_datum) {
const struct ovsdb_idl_table_class *class =
@@ -2157,8 +2161,6 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
ovsdb_idl_row_clear_old(row);
if (ovs_list_is_empty(&row->dst_arcs)) {
ovsdb_idl_row_destroy(row);
- } else {
- ovsdb_idl_row_reparse_backrefs(row);
}
}
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 44353cc..9c51dfc 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1277,6 +1277,91 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan rows, cond
010: done
]])
+dnl This test checks that deleting the destination of a reference updates the
+dnl source tracked record.
+OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, references, single delete],
+ [['["idltest",
+ {"op": "insert",
+ "table": "simple",
+ "row": {"s": "row0_s"},
+ "uuid-name": "uuid_row0_s"},
+ {"op": "insert",
+ "table": "simple6",
+ "row": {"name": "row0_s6",
+ "weak_ref": ["set",
+ [["named-uuid", "uuid_row0_s"]]
+ ]}}]']],
+ [['condition simple [true];simple6 [true]' \
+ '["idltest",
+ {"op": "delete",
+ "table": "simple",
+ "where": []}]' \
+ '["idltest",
+ {"op": "insert",
+ "table": "simple",
+ "row": {"s": "row0_s"}}]']],
+ [[000: change conditions
+001: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: inserted row: uuid=<1>
+001: inserted row: uuid=<2>
+001: name=row0_s6 weak_ref=[<1>] uuid=<2>
+001: updated columns: name weak_ref
+001: updated columns: s
+002: {"error":null,"result":[{"count":1}]}
+003: deleted row: uuid=<1>
+003: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+003: name=row0_s6 weak_ref=[] uuid=<2>
+003: updated columns: weak_ref
+004: {"error":null,"result":[{"uuid":["uuid","<3>"]}]}
+005: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3>
+005: updated columns: s
+006: done
+]])
+
+dnl This test checks that deleting both the destination and source of the
+dnl reference doesn't remove the reference the source tracked record.
+OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, references, multiple deletes],
+ [['["idltest",
+ {"op": "insert",
+ "table": "simple",
+ "row": {"s": "row0_s"},
+ "uuid-name": "uuid_row0_s"},
+ {"op": "insert",
+ "table": "simple6",
+ "row": {"name": "row0_s6",
+ "weak_ref": ["set",
+ [["named-uuid", "uuid_row0_s"]]
+ ]}}]']],
+ [['condition simple [true];simple6 [true]' \
+ '["idltest",
+ {"op": "delete",
+ "table": "simple",
+ "where": []},
+ {"op": "delete",
+ "table": "simple6",
+ "where": []}]' \
+ '["idltest",
+ {"op": "insert",
+ "table": "simple",
+ "row": {"s": "row0_s"}}]']],
+ [[000: change conditions
+001: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+001: inserted row: uuid=<1>
+001: inserted row: uuid=<2>
+001: name=row0_s6 weak_ref=[<1>] uuid=<2>
+001: updated columns: name weak_ref
+001: updated columns: s
+002: {"error":null,"result":[{"count":1},{"count":1}]}
+003: deleted row: uuid=<1>
+003: deleted row: uuid=<2>
+003: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
+003: name=row0_s6 weak_ref=[<1>] uuid=<2>
+004: {"error":null,"result":[{"uuid":["uuid","<3>"]}]}
+005: i=0 r=0 b=false s=row0_s u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3>
+005: updated columns: s
+006: done
+]])
+
OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops],
[],
[['["idltest",
More information about the dev
mailing list