On Thu, Jun 27, 2019 at 2:33 AM Ben Pfaff <[email protected]> wrote: > On Thu, Jun 27, 2019 at 12:43:18AM +0530, Numan Siddique wrote: > > On Thu, Jun 27, 2019 at 12:19 AM Ben Pfaff <[email protected]> wrote: > > > > > On Wed, Jun 26, 2019 at 07:46:25PM +0530, [email protected] wrote: > > > > From: Numan Siddique <[email protected]> > > > > > > > > Running the command "ovn-nbctl set logical_switch_port foo > > > external_ids:foo=bar" > > > > results in the incremetal processing engine to recompute the flows > on the > > > > chassis where the logical port 'foo' is claimed. > > > > > > > > This patch avoids this unnecessary recomputation by omitting the > alert > > > for the > > > > Port_Binding.external_ids column. > > > > > > > > Signed-off-by: Numan Siddique <[email protected]> > > > > Acked-by: Han Zhou <[email protected]> > > > > --- > > > > > > > > v2 -> v3 > > > > ------ > > > > * Based on the review comments from Han, dropped p2 and p3 from the > > > series. > > > > > > This only omits the alert for external-ids from one table. I think > that > > > ovn-controller should not look at the external_ids in most tables. The > > > only ones I see it dealing with are: the one on Chassis, which seems > > > legitimate (it is only setting keys there, and the behavior is > > > documented) and external_ids:dns_id in the DNS table, which makes me > > > suspicious because it appears that it's an undocumented communication > > > channel from ovn-northd to ovn-controller. I think that all the others > > > can omit alerts. > > > > > > > To sync the dns entries from nb db to sb db, we store the uuid of nb db > dns > > row in sb db. > > I will submit a doc patch for this. Thanks for pointing this out. > > OK. > > external-ids fields are intended for use by higher-level systems > (e.. ovn-northd in the case of sbdb) to keep notes on how to associate > entities in a database with corresponding higher-level entities > (e.g. nbdb, in this case). This means that the lower-level software > (e.g. ovn-controller, in this case) don't normally read the external-ids > fields since they don't have any way to interpret it. Usually, if an > external-ids key is being used to communicate between higher and lower > level systems, it is a mistake: there should be some other interface for > the purpose, such as a dedicated column. > > Agree. And looking into the ovn-controller code which uses the external_ids, I think we can remove it quite easily. I will work on it to remove the reference for dns.
Thanks Numan > > > However, why just omit alerts? If ovn-controller does not read these > > > (it should not), then they may be omitted entirely. > > > > > Right? > > > > > > > That's right. I don't understand the alert APIs properly. I will check it > > and update the patch to omit > > tracking the external_ids of all tables. > > I guess they weren't explained very well. I sent out what I hope > improves it: > https://mail.openvswitch.org/pipermail/ovs-dev/2019-June/360140.html > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
