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