Hi Terry,
Thanks for the patch, comments below.
On 3/3/21 9:24 AM, Terry Wilson wrote:
The Python IDL notification mechanism was sending a notification
for each processed update in a transaction as it was processed.
This causes isseues with multi-row change that contain references
to each other.
For example, if a Logical_Router_Port is created along with a
Gateway_Chassis, and the LRP.gateway_chassis set to that GC, then
when the notify() passes the CREATE event for the LRP, the GC will
not yet have been processed, so __getattr__ when _uuid_to_row fails
to find the GC, will return the default value for LRP.gateway_chassis
which is [].
This patch has the process_update methods return the notifications
that would be produced when a row changes, so they can be queued
and sent after all rows have been processed.
Fixes: d7d417fcddf9 ("Allow subclasses of Idl to define a notification hook")
Signed-off-by: Terry Wilson <[email protected]>
---
python/ovs/db/idl.py | 41 ++++++++++++++++++++++++-----------------
tests/ovsdb-idl.at | 22 ++++++++++++++++++++++
tests/test-ovsdb.py | 7 +++++--
3 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 5850ac7ab..4b441bd85 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
<snip>
def __process_update2(self, table, uuid, row_update):
+ """Returns Notice if a column changed, False otherwise."""
row = table.rows.get(uuid)
- changed = False
if "delete" in row_update:
if row:
del table.rows[uuid]
- self.notify(ROW_DELETE, row)
- changed = True
+ return Notice(ROW_DELETE, row)
else:
# XXX rate-limit
vlog.warn("cannot delete missing row %s from table"
@@ -680,30 +691,27 @@ class Idl(object):
self.__add_default(table, row_update)
changed = self.__row_update(table, row, row_update)
table.rows[uuid] = row
- if changed:
- self.notify(ROW_CREATE, row)
+ return Notice(ROW_CREATE, row) if changed else False
I think this should still be under the 'if changed' since it's based on
the return value of the self.__row_update() call. See below.
elif "modify" in row_update:
if not row:
raise error.Error('Modify non-existing row')
old_row = self.__apply_diff(table, row, row_update['modify'])
- self.notify(ROW_UPDATE, row, Row(self, table, uuid, old_row))
- changed = True
+ return Notice(ROW_UPDATE, row, Row(self, table, uuid, old_row))
else:
raise error.Error('<row-update> unknown operation',
row_update)
- return changed
+ return False
def __process_update(self, table, uuid, old, new):
- """Returns True if a column changed, False otherwise."""
+ """Returns Notice if a column changed, False otherwise."""
row = table.rows.get(uuid)
changed = False
if not new:
# Delete row.
if row:
del table.rows[uuid]
- changed = True
- self.notify(ROW_DELETE, row)
+ return Notice(ROW_DELETE, row)
else:
# XXX rate-limit
vlog.warn("cannot delete missing row %s from table %s"
@@ -722,8 +730,7 @@ class Idl(object):
changed |= self.__row_update(table, row, new)
if op == ROW_CREATE:
table.rows[uuid] = row
- if changed:
- self.notify(ROW_CREATE, row)
+ return Notice(ROW_CREATE, row) if changed else False
else:
op = ROW_UPDATE
if not row:
@@ -737,8 +744,8 @@ class Idl(object):
if op == ROW_CREATE:
table.rows[uuid] = row
if changed:
- self.notify(op, row, Row.from_json(self, table, uuid, old))
- return changed
+ return Notice(op, row, Row.from_json(self, table, uuid, old))
This is a similar call as above, but the return stayed under the 'if
changed' block.
-Brian
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
