On 6/3/24 06:20, Mike Pattrick wrote:
> Currently all OVSDB database queries except for UUID lookups all result
> in linear lookups over the entire table, even if an index is present.
>
> This patch modifies ovsdb_query() to attempt an index lookup first, if
> possible. If no matching indexes are present then a linear index is
> still conducted.
>
> Reported-at: https://issues.redhat.com/browse/FDP-590
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> NEWS | 3 ++
> ovsdb/query.c | 102 +++++++++++++++++++++++++++++++++++----
> ovsdb/row.h | 28 +++++++++++
> ovsdb/transaction.c | 27 -----------
> tests/ovsdb-execution.at | 34 ++++++++++++-
> tests/ovsdb-server.at | 2 +-
> tests/ovsdb-tool.at | 2 +-
> 7 files changed, 159 insertions(+), 39 deletions(-)
Hi, Mike. Thanks for the patch.
Besides what Simon asked, the patch has a few other issues:
1. Lookup is performed only on the committed index and it doesn't include
rows that are in-flight in the current transaction.
Unlike rows in a hash table, indexes are updated only after the whole
transaction is committed. With this change we'll not be able to find
newly added rows.
Another thing related to this is that it is allowed to have duplicates
within a transaction as long as they are removed before the transaction
ends. So it is possible that multiple rows will satisfy the condition
on indexed columns while the transaction is in-flight.
Consider the following commands executed in a sandbox:
# ovs-vsctl set-manager "tcp:my-first-target"
# ovsdb-client transact unix:$(pwd)/sandbox/db.sock '
["Open_vSwitch",
{"op": "select",
"table": "Manager",
"columns": ["_uuid", "target"],
"where": [["target", "==", "tcp:my-first-target"]]},
{"op": "insert",
"table": "Manager",
"uuid-name": "duplicate",
"row": {"target": "tcp:my-first-target"}},
{"op": "select",
"table": "Manager",
"columns": ["_uuid", "target"],
"where": [["target", "==", "tcp:my-first-target"]]},
{"op": "delete",
"table": "Manager",
"where":[["_uuid","==",["named-uuid","duplicate"]]]},
{"op": "select",
"table": "Manager",
"columns": ["_uuid", "target"],
"where": [["target", "==", "tcp:my-first-target"]]}]'
Transaction must succeed. The first selection should return 1 row,
the second should return both duplicates and the third should again
return one row.
Ideally, implementation should not leak the transaction details to
the query module, though I'm not sure if that is 100% achievable.
2. Taking above case into account, this change needs way more unit tests
with different order of operations and complex data updates.
3. Since this is a performance-oriented change, please, include some
performance numbers in the commit message as well, including impact
on non-indexed lookups, if any.
4. There seems to be a lot of logic overlap with existing functions like
ovsdb_condition_match_every_clause(), ovsdb_index_search() and
ovsdb_row_hash_columns(). Can we re-use those instead? For example,
by creating a row from the conditions before the lookup? What a
performance impact will look like?
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
