On 2/19/25 16:11, Max Lamprecht wrote: > On Wed, Feb 19, 2025 at 02:48:10PM +0100, Ilya Maximets wrote: >> On 2/13/25 16:39, Max Lamprecht via dev wrote: >>> This hook can be used by libaries to restore previous indexes after a >>> clear. >>> >>> Signed-off-by: Max Lamprecht <[email protected]> >>> --- >>> python/ovs/db/idl.py | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>> index c8cc54346..03efb1a01 100644 >>> --- a/python/ovs/db/idl.py >>> +++ b/python/ovs/db/idl.py >>> @@ -790,6 +790,9 @@ class Idl(object): >>> to doing nothing to avoid overhead where it is not needed. >>> """ >>> >>> + def after_clear(self): >>> + """Hook that can be used to restore indexes after a clear.""" >>> + >>> def __clear(self): >>> changed = False >>> >>> @@ -802,6 +805,7 @@ class Idl(object): >>> >>> if changed: >>> self.change_seqno += 1 >>> + self.after_clear() >>> >>> def __update_has_lock(self, new_has_lock): >>> if new_has_lock and not self.has_lock: >>> -- >>> 2.43.0 >> >> Hi, Max. Thanks for the patch! Could you please explain in more details >> what's the use case for this functionality? It's not obvious to me why >> such a hook is needed. >> >> Best regards, Ilya Maximets. > Hi Ilya, > > We use openstack in combination with ovn/ovs. > We observed that our networking api(neutron) got slower over time and > discovered that the > ovsidl loses indexes. > > We noticed that right before we see the lookups without indices, we called > self.clear() in ovs [1] > It seems like the idl called for a new full sync(comment: "clear table rows > for new dump") > With "table.rows = custom_index.IndexedRows(table)" this clear() function > reinitializes > the complete table.rows object which also clears all indixes which where > created by the ovsdbapp lib [2] > > After this patch I will submit another patch to ovsdbapp that uses this > hook to restore previous indexes.
Thanks for the explanation. I think, I understand the problem now, but I'm not sure if we need a separate callback for this. The clearing of the database happens on re-connections if a new data snapshot is received from the database (e.g. because the current one is too old and the server couldn't send an incremental update). So, IDL clears all the old data and adds all the nex data from the new snapshot. C applications like ovn-controller detect such changes by checking condition_seqno and triggering a re-compute. Change tracking in the python IDL is a bit different, so I understand why we may want to handle this situation via a hook, since other change notifications are done this way. But I'm nto sure about it being a separate hook. A few potential solutions for the situation: 1. Track cond_seqno in ovsdbapp and recover caches when it changes. This would be similar to what C applications like ovn-controller do. But may be less convenient for python applications that already use notification hooks. And we may need to remove the setting it to zero in the _clear() call (it's strange that we clear it, C version doesn't do that). 2. Notify row deletion for all the rows and then parsing of the new data will notify addition of all the rows once the new snapshot is parsed. This seems logical, but may be confusing for the application, since the rows are not actually deleted from the database, we just report that temporarily. Applications will need to act on that though as if the row was removed, so it will create a lot of churn and potentially unwanted side effects. So, we really shouldn't do that. 3. Notify about the database reset once. This is this patch. However, instead of adding a new hook, maybe we can add a new event type for the notify() hook? Something like "DB_CLEAR" or "DB_RESET" ? This event will not come with the removed rows, obviously. The application can use this notification to clear its caches or any other data and prepare for new re-parsed data to come. What do you think? Best regards, Ilya Maximets. > > [1] > https://github.com/openvswitch/ovs/blob/8b7f1eb8db1aa99ccf7b542662129450caff65e0/python/ovs/db/idl.py#L495 > [2] > https://github.com/openstack/ovsdbapp/blob/14300779028fab5b1756dc35544d08ec0bb1f1b4/ovsdbapp/backend/ovs_idl/__init__.py#L81 > [3] https://bugs.launchpad.net/neutron/+bug/2092133/comments/5 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
