[ovs-discuss] OVN: Any objections for making logical router and logical switches tables indexed by name?

Flaviof flavio at flaviof.com
Sat Aug 14 20:09:05 UTC 2021


[cc + Dave and Terry]

On Thu, Aug 12, 2021 at 2:19 PM Han Zhou <hzhou at ovn.org> wrote:

>
>
> On Tue, Aug 10, 2021 at 9:32 AM Flavio Fernandes <flavio at flaviof.com>
> wrote:
> >
> > [cc: Ben + Dmitry]
> >
> > Hi folks,
> >
> > I'm looking at some conversion code in ovn-org/ovn-kubernetes where we
> replace the ovn-nbctl wrapper with the libovsdb library ( ovn-org/libovsdb
> ). Since we are mostly doing this to make it faster (besides reducing the
> memory footprint), using the "Where" syntax [1] will greatly benefit
> operations on logical-router [2] and logical-switch [3] tables if they were
> indexed by name. Similar to what we already have for the logical-router
> port and logical-switch port tables.
> >
> > After listening to episode 72 of OVS Orbit [4], I would like to ask:
> does anyone have objections to adding "indexes": [["name"]],  to the
> logical-router [2] and logical-switch [3] tables? I understand Ben's point
> on making the implementation of the locally-cached tables have these types
> of optimizations, but at the same time, I see these 2 tables as low-hanging
> fruits when scaling deployments with lots of lr's and ls's. Unless there is
> an implementation that use nameless rows for these tables, I cannot think
> of a usage case where duplicate names are useful. Do you?
> >
> > Depending on your answers, I can propose a tweak to the schema to have
> these changes... or not. ;)
> >
> > Thanks,
> >
> > -- flaviof
> >
> > [1]:  https://github.com/ovn-org/libovsdb/pull/209
> > [2]:
> https://github.com/ovn-org/ovn/blob/d08f89e219e1fa45583757bd2804783cf0630179/ovn-nb.ovsschema#L306
> > [3]:
> https://github.com/ovn-org/ovn/blob/d08f89e219e1fa45583757bd2804783cf0630179/ovn-nb.ovsschema#L41
> > [4]: https://ovsorbit.org/ ==> Episode 72: The OVSDB Query Optimizer
> and Key-Value Interface, with Dmitry Yusupov from NVIDIA (Feb 27, 2021)
> > _______________________________________________
> > discuss mailing list
> > discuss at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
>
> I have no objection to the schema change, as long as it is declared as
> incompatible change in the NEWS and add some guide in the upgrading topics
> in the documentation regarding how to solve the problem when upgrading.
>
> However I don't see a link between this and [4]. The talk was about server
> side indexes which optimizes “select“ operations while what libovsdb does
> is on the local cache. The pr [1] seems could utilize the index in the
> schema but is not related to the server side indexes.
>
> In addition I would propose that libovsdb add support for adding
> customized indexes with some new APIs (of course not tied to any schema),
> not only on primary keys but other columns as needed by clients, and then
> schema wouldn’t matter that much for the cache indexes.
>

You are absolutely right, Ben & Han!

The only 'connection' between this and [4] is that libovsdb is smart in
indexing rows in its local cache based
on the schema. Not a good enough reason to break backwards compatibility.
@Dave: please correct me if I'm
talking nonsense here. :)

So, after thinking some more about this, I agree that making libovsdb
capable of indexing additional columns
not mentioned in the schema is indeed a much better approach. I also asked
Terry and looked at what we
currently do in ml2/ovn, via ovsdbapp [5] [6]. Since LS and LR in ovn-k8
use unique names (same as in Neutron),
that enhancement may not even have to deal with duplicates.

Thank you so much for your (and blp's) feedback on this !!!

-- flaviof

[5]:  https://review.opendev.org/#/c/678076/
[6]:
https://github.com/openstack/ovsdbapp/blob/35c8eae020ef5300e08b7869e5da11dd0e2c500e/ovsdbapp/schema/ovn_northbound/impl_idl.py#L24-L26




>
> Thanks,
> Han
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-discuss/attachments/20210814/2e889aca/attachment.html>


More information about the discuss mailing list