On 1/5/24 04:13, Tao Liu wrote: > > On 1/4/24 7:56 PM, Ilya Maximets wrote: >> On 7/4/23 12:41, Tao Liu wrote: >>> Hi, Ilya. Thanks for your comment. >>> >>> On 7/4/23 4:10 AM, Ilya Maximets wrote: >>>> On 7/2/23 11:59, Tao Liu wrote: >>>>> Commit 02f31a1262fc has fixed the row references when insertion and >>>>> deletion execute in one IDL run. However, we can still hit ovn-controller >>>>> crash in pressure test where pb->datapath == NULL. It is triggered by >>>>> reference dst deletion in same IDL run: >>>>> >>>>> idl run: >>>>> update1: >>>>> insert port_binding >>>>> insert datapath_binding >>>>> update2: >>>>> delete datapath_binding >>>>> delete port_binding >>>>> >>>>> Fix it by reparse reference src before dst deletion. >>>> Hi. Thanks for the patch! >>>> >>>> There seems to be an issue indeed. Removed rows should have valid >>>> references to other removed rows. >>>> >>>> However, I'm not sure this solution solves the issue in a general case >>>> as it depends on the order of events. If events in the update2 will >>>> be in opposite order, the issue will still be there. >>> The opposite order of update2 has been fixed in commit 02f31a1262fc, >>> which now moved to >>> >>> first reparse_row(). >>> >>>> In a more general case we may also have two rows reference each other >>>> and we need to preserve both references. >>>> >>>> We probably could: >>>> >>>> 1. Postpone all the deletions, execute insertions and updates. >>>> >>>> 2. Call ovsdb_idl_reparse_refs_to_inserted() to re-parse every row >>>> that needs re-parsing due to newly added rows. >>>> >>>> 3. Delete the rows that need to be deleted. >>>> >>>> 4. Call ovsdb_idl_reparse_deleted() to get rid of references to >>>> deleted rows from non-deleted ones. >>> Sounds like a good idea, that makes things more clear. >>> >>>> What do you think? >>>> Dumitru, maybe you have some thoughts on this? >>>> >>>> And we will need need a unit test for this issue. >>> Will try to add one. >> Hi, Tao Liu. >> >> Are you still working on a fix for this issue? Or maybe someone >> else should take a look into that? >> >> Best regards, Ilya Maximets. > > > Hi, Ilya > > I'm not going on a new version. It would be appreciate if you can help > to continue this fix. > > Best regards, Tao
OK. No problem. We'll find someone to work on this, or I'll try to find some time for this myself. For the record, I'm not sure the sequence of steps I proposed above is fully correct, since the order between insertions and deletions might be significant, if they come from different updates. So, we may have to batch re-parsing (i.e. re-parse accumulated refs to deleted when we encounter an insertion and re-parse accumulated refs to inserted when we encounter a deletion), but that needs some more thinking. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
