Re: [ovs-dev] [PATCH 2/3] python: ovsdb-idl: Make IndexedRows mirror hmap.
On 4/11/24 15:57, Terry Wilson wrote: > I tried to get this to thread under the "ovsdb-idl: potential issues > with the persistent UUID implementation" thread, but failed. This is > one potential solution to that which mirrors the current C IDL > implementation which seems to work for the most part, but relying on > the fact that hmaps allow duplicate keys wasn't necessarily intended > there. Reading that thread is definitely a prerequisite for > understanding why this patch exists. And it may be better to solve > this some other way, but it seems difficult to do for the case of > "inserting a UUID that already exists" w/o creating a race condition > with multiple IDL clients. Thanks, Terry! As discussed off list, we need to think a bit more how to change the API, so it is less awkward. For now the current fix makes sense to me. Applied remaining 2 patches and backported them down to 3.1. Best regards, Ilya Maximets. > > Terry > > On Wed, Apr 10, 2024 at 4:39 PM Terry Wilson wrote: >> >> The Python IDL code very closely mirrors the C IDL code, which uses >> an hmap to store table rows. hmap code allows duplicate keys, while >> IndexedRows, which is derived from DictBase does not. >> >> The persistent UUID code can attempt to temporarily add a Row with >> a duplicate UUID to table.rows, so IndexedRows is modified to >> behave similarly to the C IDL's hmap implementation. >> >> Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row >> insert.") >> Signed-off-by: Terry Wilson >> --- >> python/ovs/db/custom_index.py | 13 ++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/python/ovs/db/custom_index.py b/python/ovs/db/custom_index.py >> index 587caf5e3..3fa03d3c9 100644 >> --- a/python/ovs/db/custom_index.py >> +++ b/python/ovs/db/custom_index.py >> @@ -90,14 +90,21 @@ class IndexedRows(DictBase, object): >> index = self.indexes[name] = MultiColumnIndex(name) >> return index >> >> +def __getitem__(self, key): >> +return self.data[key][-1] >> + >> def __setitem__(self, key, item): >> -self.data[key] = item >> +try: >> +self.data[key].append(item) >> +except KeyError: >> +self.data[key] = [item] >> for index in self.indexes.values(): >> index.add(item) >> >> def __delitem__(self, key): >> -val = self.data[key] >> -del self.data[key] >> +val = self.data[key].pop() >> +if len(self.data[key]) == 0: >> +del self.data[key] >> for index in self.indexes.values(): >> index.remove(val) >> >> -- >> 2.34.3 >> > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/3] python: ovsdb-idl: Make IndexedRows mirror hmap.
I tried to get this to thread under the "ovsdb-idl: potential issues with the persistent UUID implementation" thread, but failed. This is one potential solution to that which mirrors the current C IDL implementation which seems to work for the most part, but relying on the fact that hmaps allow duplicate keys wasn't necessarily intended there. Reading that thread is definitely a prerequisite for understanding why this patch exists. And it may be better to solve this some other way, but it seems difficult to do for the case of "inserting a UUID that already exists" w/o creating a race condition with multiple IDL clients. Terry On Wed, Apr 10, 2024 at 4:39 PM Terry Wilson wrote: > > The Python IDL code very closely mirrors the C IDL code, which uses > an hmap to store table rows. hmap code allows duplicate keys, while > IndexedRows, which is derived from DictBase does not. > > The persistent UUID code can attempt to temporarily add a Row with > a duplicate UUID to table.rows, so IndexedRows is modified to > behave similarly to the C IDL's hmap implementation. > > Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row > insert.") > Signed-off-by: Terry Wilson > --- > python/ovs/db/custom_index.py | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/python/ovs/db/custom_index.py b/python/ovs/db/custom_index.py > index 587caf5e3..3fa03d3c9 100644 > --- a/python/ovs/db/custom_index.py > +++ b/python/ovs/db/custom_index.py > @@ -90,14 +90,21 @@ class IndexedRows(DictBase, object): > index = self.indexes[name] = MultiColumnIndex(name) > return index > > +def __getitem__(self, key): > +return self.data[key][-1] > + > def __setitem__(self, key, item): > -self.data[key] = item > +try: > +self.data[key].append(item) > +except KeyError: > +self.data[key] = [item] > for index in self.indexes.values(): > index.add(item) > > def __delitem__(self, key): > -val = self.data[key] > -del self.data[key] > +val = self.data[key].pop() > +if len(self.data[key]) == 0: > +del self.data[key] > for index in self.indexes.values(): > index.remove(val) > > -- > 2.34.3 > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/3] python: ovsdb-idl: Make IndexedRows mirror hmap.
The Python IDL code very closely mirrors the C IDL code, which uses an hmap to store table rows. hmap code allows duplicate keys, while IndexedRows, which is derived from DictBase does not. The persistent UUID code can attempt to temporarily add a Row with a duplicate UUID to table.rows, so IndexedRows is modified to behave similarly to the C IDL's hmap implementation. Fixes: 55b9507e6824 ("ovsdb-idl: Add the support to specify the uuid for row insert.") Signed-off-by: Terry Wilson --- python/ovs/db/custom_index.py | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/python/ovs/db/custom_index.py b/python/ovs/db/custom_index.py index 587caf5e3..3fa03d3c9 100644 --- a/python/ovs/db/custom_index.py +++ b/python/ovs/db/custom_index.py @@ -90,14 +90,21 @@ class IndexedRows(DictBase, object): index = self.indexes[name] = MultiColumnIndex(name) return index +def __getitem__(self, key): +return self.data[key][-1] + def __setitem__(self, key, item): -self.data[key] = item +try: +self.data[key].append(item) +except KeyError: +self.data[key] = [item] for index in self.indexes.values(): index.add(item) def __delitem__(self, key): -val = self.data[key] -del self.data[key] +val = self.data[key].pop() +if len(self.data[key]) == 0: +del self.data[key] for index in self.indexes.values(): index.remove(val) -- 2.34.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev