[ovs-dev] [PATCH v2] python: Send notifications after the transaction ends

Brian Haley haleyb.dev at gmail.com
Wed Mar 3 19:47:36 UTC 2021


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 <twilson at redhat.com>
> ---
>   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


More information about the dev mailing list