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 <m...@redhat.com> > --- > 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev