On Tue, Jun 11, 2019 at 12:12 PM Dumitru Ceara <[email protected]> wrote: > > On Fri, Jun 7, 2019 at 10:42 PM Ben Pfaff <[email protected]> wrote: > > > > On Wed, May 22, 2019 at 01:26:03PM +0200, Dumitru Ceara wrote: > > > The chassis_run code didn't take into account the scenario when the > > > system-id was changed in the Open_vSwitch table. Due to this the code > > > was trying to insert a new Chassis record in the OVN_Southbound DB with > > > the same Encaps as the previous Chassis record. The transaction used > > > to insert the new records was aborting due to the ["type", "ip"] > > > index constraint violation as we were creating new Encap entries with > > > the same "type" and "ip" as the old ones. > > > > > > In order to fix this issue the flow is now: > > > 1. the first time ovn-controller initializes the Chassis entry (shortly > > > after start up) we first check if there is a stale Chassis record in the > > > OVN_Southbound DB by checking if any of the old Encap entries associated > > > to the Chassis record match the new tunnel configuration. If found it > > > means that ovn-controller didn't shutdown gracefully last time it was > > > run so it didn't cleanup the Chassis table. Potentially in the meantime > > > the OVS system-id was also changed. We then update the stale entry with > > > the new configuration and store the last configured chassis-id in memory > > > to avoid walking the Chassis table every time. > > > 2. for subsequent chassis_run calls we use the last configured > > > chassis-id stored at the previous step to lookup the old Chassis record. > > > 3. when ovn-controller shuts down gracefully we lookup the Chassis > > > record based on the chassis-id stored in memory at steps 1 and 2 above. > > > This is to avoid failing to cleanup the Chassis record in OVN_Southbound > > > DB if the OVS system-id changes between the last call to chassis_run and > > > chassis_cleanup. > > > > > > With this commit we also: > > > - refactor chassis.c to abstract the string processing and use > > > library data structures (e.g., sset) > > > - rename the get_chassis_id function in ovn-controller.c to > > > get_ovs_chassis_id to avoid confusion with the newly added > > > chassis_get_id function from chassis.c which returns the last > > > successfully configured chassis-id. > > > - add a test case in ovn-controller.at to check that OVS system-id > > > changes are properly propagated to OVN_Southbound DB > > > > Thanks for working on this. > > > > This is a large patch that incorporates both a bug fix and refactoring. > > I would prefer to see the refactoring broken out into a separate patch. > > Preferably, it would go after the bug fix so that the bug fix could be > > backported by itself (if necessary), but if the refactoring is essential > > to the bug fix then the refactoring could go first. > > > > Would you mind breaking the patch apart? > > I'll try to split this in two patches.
Hi Ben, In the end I was in a situation that mixes the two cases you mentioned above: - the fix for most cases (also the one reported in the bug tracker) doesn't need the refactoring - the fix for the case when ovn-controller dies and restarts needed the refactoring. So I decided to send a series of 3 patches (main fix + refactoring + final fix): https://patchwork.ozlabs.org/project/openvswitch/list/?series=113354 Is it ok like this or do you prefer a separate patch for the main fix? Thanks, Dumitru > > Thanks, > Dumitru > > > > > Thanks, > > > > Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
