I'll take a look today. Might take me a little bit as python-ovs and
the indexing code isn't always the easiest to review.

On Fri, Oct 4, 2024 at 5:35 AM Eelco Chaudron <[email protected]> wrote:
>
>
>
> On 4 Oct 2024, at 12:23, Eelco Chaudron wrote:
>
> > On 1 Oct 2024, at 11:20, Luca Czesla via dev wrote:
> >
> >> For an index on a reference to work, we should not resolve the reference to
> >> to the corresponding row object but instead use the uuid for indexing.
> >>
> >> an index on Datapath in the Port_Binding Table reduces the lookup in the
> >> neutron metadata agent for each request from approx. 400ms per request to
> >> 980μs with approx. 40k port bindings.
> >>
> >> Signed-off-by: Luca Czesla <[email protected]>
> >
> >
> > Hi Luca,
> >
> > The v2 looks good to me.
> >
> > //Eelco
> >
> > Acked-by: Eelco Chaudron <[email protected]>
>
>
> In addition, Terry would you be able to have a look before we apply the patch?
>
> Also, we might need to adjust the title a bit, probably something like:
>
>   python: Use index references in the MultiColumnIndex class.
>
> Cheers,
>
> Eelco
>
> >> ---
> >>  python/ovs/db/custom_index.py | 14 +++++++++++---
> >>  python/ovs/db/idl.py          |  9 ++++++---
> >>  2 files changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/python/ovs/db/custom_index.py b/python/ovs/db/custom_index.py
> >> index 3fa03d3c9..803baaccd 100644
> >> --- a/python/ovs/db/custom_index.py
> >> +++ b/python/ovs/db/custom_index.py
> >> @@ -11,7 +11,7 @@ try:
> >>  except ImportError:
> >>      from ovs.compat import sortedcontainers
> >>
> >> -from ovs.db import data
> >> +from ovs.db import data, idl
> >>
> >>  OVSDB_INDEX_ASC = "ASC"
> >>  OVSDB_INDEX_DESC = "DESC"
> >> @@ -43,7 +43,7 @@ class MultiColumnIndex(object):
> >>
> >>      def _cmp(self, a, b):
> >>          for col, direction, key in self.columns:
> >> -            aval, bval = key(a), key(b)
> >> +            aval, bval = idl._row_to_uuid(key(a)), 
> >> idl._row_to_uuid(key(b))
> >>              if aval == bval:
> >>                  continue
> >>              result = (aval > bval) - (aval < bval)
> >> @@ -53,7 +53,8 @@ class MultiColumnIndex(object):
> >>      def index_entry_from_row(self, row):
> >>          return row._table.rows.IndexEntry(
> >>              uuid=row.uuid,
> >> -            **{c.column: getattr(row, c.column) for c in self.columns})
> >> +            **{c.column: row.get_column(c.column, self._uuid_to_uuid)
> >> +                for c in self.columns})
> >>
> >>      def add(self, row):
> >>          if not all(hasattr(row, col.column) for col in self.columns):
> >> @@ -76,6 +77,13 @@ class MultiColumnIndex(object):
> >>      def __iter__(self):
> >>          return iter(r._table.rows[r.uuid] for r in self.values)
> >>
> >> +    # For indexing we do not want to resolve the references as we cannot
> >> +    # ensure that the referenced table is already filled, the uuid is
> >> +    # sufficient for the index.
> >> +    @staticmethod
> >> +    def _uuid_to_uuid(atom, base):
> >> +        return atom
> >> +
> >>
> >>  class IndexedRows(DictBase, object):
> >>      def __init__(self, table, *args, **kwargs):
> >> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> >> index c8cc54346..3feb068b8 100644
> >> --- a/python/ovs/db/idl.py
> >> +++ b/python/ovs/db/idl.py
> >> @@ -1351,7 +1351,7 @@ class Row(object):
> >>          else:
> >>              return atom
> >>
> >> -    def __getattr__(self, column_name):
> >> +    def get_column(self, column_name, uuid_to_row):
> >>          assert self._changes is not None
> >>          assert self._mutations is not None
> >>
> >> @@ -1392,7 +1392,7 @@ class Row(object):
> >>                      datum = data.Datum.from_python(column.type, dlist,
> >>                                                     _row_to_uuid)
> >>                  elif column.type.is_map():
> >> -                    dmap = datum.to_python(self._uuid_to_row)
> >> +                    dmap = datum.to_python(uuid_to_row)
> >>                      if inserts is not None:
> >>                          dmap.update(inserts)
> >>                      if removes is not None:
> >> @@ -1409,7 +1409,10 @@ class Row(object):
> >>                  else:
> >>                      datum = inserts
> >>
> >> -        return datum.to_python(self._uuid_to_row)
> >> +        return datum.to_python(uuid_to_row)
> >> +
> >> +    def __getattr__(self, column_name):
> >> +        return self.get_column(column_name, self._uuid_to_row)
> >>
> >>      def __setattr__(self, column_name, value):
> >>          assert self._changes is not None
> >>
> >> base-commit: 1b99649025ae767974d0a7cea88d1e9f46f4378d
> >> --
> >> 2.39.5
> >>
> >> _______________________________________________
> >> dev mailing list
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to