On Thu, Jun 6, 2024 at 10:41 AM Dumitru Ceara <[email protected]> wrote: > > On 5/27/24 23:39, Ilya Maximets wrote: > > When a row is modified, python IDL doesn't perform any operations on > > existing client-side indexes. This means that if the column on which > > index is created changes, the old value will remain in the index and > > the new one will not be added to the index. Beside lookup failures > > this is also causing inability to remove modified rows, because the > > new column value doesn't exist in the index causing an exception on > > attempt to remove it: > > > > Traceback (most recent call last): > > File "ovsdbapp/backend/ovs_idl/connection.py", line 110, in run > > self.idl.run() > > File "ovs/db/idl.py", line 465, in run > > self.__parse_update(msg.params[2], OVSDB_UPDATE3) > > File "ovs/db/idl.py", line 924, in __parse_update > > self.__do_parse_update(update, version, self.tables) > > File "ovs/db/idl.py", line 964, in __do_parse_update > > changes = self.__process_update2(table, uuid, row_update) > > File "ovs/db/idl.py", line 991, in __process_update2 > > del table.rows[uuid] > > File "ovs/db/custom_index.py", line 102, in __delitem__ > > index.remove(val) > > File "ovs/db/custom_index.py", line 66, in remove > > self.values.remove(self.index_entry_from_row(row)) > > File "sortedcontainers/sortedlist.py", line 2015, in remove > > raise ValueError('{0!r} not in list'.format(value)) > > ValueError: Datapath_Binding( > > uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list > > > > Fix that by always removing an existing row from indexes before > > modification and adding back afterwards. This ensures that old > > values are removed from the index and new ones are added. > > > > This behavior is consistent with the C implementation. > > > > The new test that reproduces the removal issue is added. Some extra > > testing infrastructure added to be able to handle and print out the > > 'indexed' table from the idltest schema. > > > > Fixes: 13973bc41524 ("Add multi-column index support for the Python IDL") > > Reported-at: > > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-May/053159.html > > Reported-by: Roberto Bartzen Acosta <[email protected]> > > Signed-off-by: Ilya Maximets <[email protected]> > > --- > > Looks good to me: > > Acked-by: Dumitru Ceara <[email protected]> > > Regards, > Dumitru >
Looks good to me. I don't like that my code for IndexedRows strongly implies that it behaves exactly like a dict, and in this case it doesn't. Maybe some comments explaining why a delete has to be done for posterity would be helpful. Acked-by: Terry Wilson <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
