Merge "Add indexed column support to ovsdbapp"

ovsdbapp/backend/ovs_idl/__init__.py

@@ -24,33 +24,97 @@ _NO_DEFAULT = object()
 
 class Backend(object):
     lookup_table = {}
-    ovsdb_connection = None
+    _ovsdb_connection = None
 
-    def __init__(self, connection, start=True, **kwargs):
+    def __init__(self, connection, start=True, auto_index=True, **kwargs):
         super(Backend, self).__init__(**kwargs)
+        self.ovsdb_connection = connection
+        if auto_index:
+            self.autocreate_indices()
         if start:
             self.start_connection(connection)
 
-    @classmethod
-    def start_connection(cls, connection):
+    @property
+    def ovsdb_connection(self):
+        return self.__class__._ovsdb_connection
+
+    @ovsdb_connection.setter
+    def ovsdb_connection(self, connection):
+        if self.__class__._ovsdb_connection is None:
+            self.__class__._ovsdb_connection = connection
+
+    def create_index(self, table, *columns):
+        """Create a multi-column index on a table
+
+        :param table:   The table on which to create an index
+        :type table:    string
+        :param columns: The columns in the index
+        :type columns:  string
+        """
+
+        index_name = idlutils.index_name(*columns)
+        idx = self.tables[table].rows.index_create(index_name)
+        idx.add_columns(*columns)
+
+    def autocreate_indices(self):
+        """Create simple one-column indexes
+
+        This creates indexes for all lookup_table entries and for all defined
+        indexes columns in the OVSDB schema, as long as they are simple
+        one-column indexes (e.g. 'name') fields.
+        """
+
+        tables = set(self.idl.tables.keys())
+        # lookup table indices
+        for table, (lt, col, uuid_col) in self.lookup_table.items():
+            if table != lt or not col or uuid_col or table not in tables:
+                # Just handle simple cases where we are looking up a single
+                # column on a single table
+                continue
+            index_name = idlutils.index_name(col)
+            try:
+                idx = self.idl.tables[table].rows.index_create(index_name)
+            except ValueError:
+                # index already exists
+                pass
+            else:
+                idx.add_column(col)
+                LOG.debug("Created index %s", index_name)
+            tables.remove(table)
+
+        # Simple ovsdb-schema indices
+        for table in self.idl.tables.values():
+            if table.name not in tables:
+                continue
+            col = idlutils.get_index_column(table)
+            if not col:
+                continue
+            index_name = idlutils.index_name(col)
+            try:
+                idx = table.rows.index_create(index_name)
+            except ValueError:
+                pass  # index already exists
+            else:
+                idx.add_column(col)
+                LOG.debug("Created index %s", index_name)
+            tables.remove(table.name)
+
+    def start_connection(self, connection):
         try:
-            if cls.ovsdb_connection is None:
-                cls.ovsdb_connection = connection
-                cls.ovsdb_connection.start()
+            self.ovsdb_connection.start()
         except Exception as e:
             connection_exception = exceptions.OvsdbConnectionUnavailable(
-                db_schema=cls.schema, error=e)
+                db_schema=self.schema, error=e)
             LOG.exception(connection_exception)
             raise connection_exception
 
-    @classmethod
-    def restart_connection(cls):
-        cls.ovsdb_connection.stop()
-        cls.ovsdb_connection.start()
+    def restart_connection(self):
+        self.ovsdb_connection.stop()
+        self.ovsdb_connection.start()
 
     @property
     def idl(self):
-        return self.__class__.ovsdb_connection.idl
+        return self.ovsdb_connection.idl
 
     @property
     def tables(self):
@@ -60,8 +124,8 @@ class Backend(object):
 
     def create_transaction(self, check_error=False, log_errors=True, **kwargs):
         return transaction.Transaction(
-            self, self.__class__.ovsdb_connection,
-            self.__class__.ovsdb_connection.timeout,
+            self, self.ovsdb_connection,
+            self.ovsdb_connection.timeout,
             check_error, log_errors)
 
     def db_create(self, table, **col_values):
@@ -119,15 +183,18 @@ class Backend(object):
             return record.result
 
         t = self.tables[table]
-        try:
-            if isinstance(record, uuid.UUID):
-                return t.rows[record]
+        if isinstance(record, uuid.UUID):
             try:
-                uuid_ = uuid.UUID(record)
-                return t.rows[uuid_]
-            except ValueError:
-                # Not a UUID string, continue lookup by other means
-                pass
+                return t.rows[record]
+            except KeyError:
+                raise idlutils.RowNotFound(table=table, col='uuid',
+                                           match=record)
+        try:
+            uuid_ = uuid.UUID(record)
+            return t.rows[uuid_]
+        except ValueError:
+            # Not a UUID string, continue lookup by other means
+            pass
         except KeyError:
             # If record isn't found by UUID , go ahead and look up by the table
             pass

ovsdbapp/backend/ovs_idl/idlutils.py

@@ -54,12 +54,42 @@ class RowNotFound(exceptions.OvsdbAppException):
     message = "Cannot find %(table)s with %(col)s=%(match)s"
 
 
+def index_name(*columns):
+    assert columns
+    return "_".join(sorted(columns))
+
+
+def index_lookup(table, **matches):
+    """Find a value in Table by index
+
+    :param table:   The table to search in
+    :type table:    ovs.db.schema.TableSchema
+    :param matches: The column/value pairs of the index to search
+    :type matches:  The types of the columns being matched
+    :returns:       A Row object
+    """
+    idx = table.rows.indexes[index_name(*matches.keys())]
+    search = table.rows.IndexEntry(**matches)
+    return next(idx.irange(search, search))
+
+
+def table_lookup(table, column, match):
+    return next(r for r in table.rows.values() if getattr(r, column) == match)
+
+
 def row_by_value(idl_, table, column, match, default=_NO_DEFAULT):
     """Lookup an IDL row in a table by column/value"""
     tab = idl_.tables[table]
-    for r in tab.rows.values():
-        if getattr(r, column) == match:
-            return r
+    try:
+        return index_lookup(tab, **{column: match})
+    except KeyError:  # no index column
+        try:
+            return table_lookup(tab, column, match)
+        except StopIteration:
+            pass
+    except StopIteration:  # match not found via index
+        pass
+
     if default is not _NO_DEFAULT:
         return default
     raise RowNotFound(table=table, col=column, match=match)

ovsdbapp/schema/ovn_southbound/impl_idl.py

@@ -22,9 +22,6 @@ class OvnSbApiIdlImpl(ovs_idl.Backend, api.API):
         'Chassis': idlutils.RowLookup('Chassis', 'name', None),
     }
 
-    def __init__(self, connection):
-        super(OvnSbApiIdlImpl, self).__init__(connection)
-
     def chassis_add(self, chassis, encap_types, encap_ip, may_exist=False,
                     **columns):
         return cmd.ChassisAddCommand(self, chassis, encap_types, encap_ip,

ovsdbapp/tests/functional/backend/ovs_idl/test_indexing.py

@@ -0,0 +1,77 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from unittest import mock
+
+from ovsdbapp.backend.ovs_idl import idlutils
+from ovsdbapp.schema.ovn_northbound import impl_idl
+from ovsdbapp.tests.functional import base
+from ovsdbapp.tests import utils
+
+
+class TestOvnNbIndex(base.FunctionalTestCase):
+    schemas = ['OVN_Northbound']
+
+    def setUp(self):
+        super(TestOvnNbIndex, self).setUp()
+        self.api = impl_idl.OvnNbApiIdlImpl(self.connection)
+
+    def test_find(self):
+        # This test will easily time out if indexing isn't used
+        length = 2000
+        basename = utils.get_rand_device_name('testswitch')
+        with self.api.transaction(check_error=True) as txn:
+            for i in range(length):
+                txn.add(self.api.ls_add("%s%d" % (basename, i)))
+        match = "%s%d" % (basename, length / 2)
+        sw = self.api.lookup('Logical_Switch', match)
+        self.assertEqual(sw.name, match)
+
+    def test_default_indices(self):
+        self.assertTrue(self.api.lookup_table)
+        for key, (table, col, _) in self.api.lookup_table.items():
+            idl_table = self.api.tables[table]
+            self.assertIn(col, idl_table.rows.indexes)
+
+
+class TestOvnNbWithoutIndex(base.FunctionalTestCase):
+    schemas = ['OVN_Northbound']
+
+    # Due to ovsdbapp by default creating singleton connections, it's possible
+    # that a test is run where we already have a connection/idl set up that
+    # already has indexing set up on it.
+    class NewNbApiIdlImpl(impl_idl.OvnNbApiIdlImpl):
+        @property
+        def ovsdb_connection(self):
+            return self._ovsdb_connection
+
+        @ovsdb_connection.setter
+        def ovsdb_connection(self, connection):
+            self._ovsdb_connection = connection
+
+    def setUp(self):
+        super(TestOvnNbWithoutIndex, self).setUp()
+        self.api = self.NewNbApiIdlImpl(self.connection, start=False,
+                                        auto_index=False)
+
+    @mock.patch.object(idlutils, 'table_lookup')
+    def test_create_index(self, table_lookup):
+        self.assertFalse(self.api.tables['Logical_Switch'].rows.indexes)
+        self.api.create_index("Logical_Switch", "name")
+        self.api.start_connection(self.connection)
+        name = utils.get_rand_device_name("testswitch")
+        self.api.ls_add(name).execute(check_error=True)
+        sw = self.api.lookup('Logical_Switch', name)
+        self.assertEqual(sw.name, name)
+        self.assertRaises(idlutils.RowNotFound, self.api.lookup,
+                          'Logical_Switch', 'nothere')
+        table_lookup.assert_not_called()

ovsdbapp/tests/unit/backend/ovs_idl/test_idlutils.py

@@ -169,3 +169,15 @@ class TestIdlUtils(base.TestCase):
                                      mock.sentinel.table_name,
                                      FAKE_RECORD_GUID)
         self.assertEqual(mock.sentinel.row_value, res)
+
+    def test_index_name(self):
+        expected = {
+            ('one',): 'one',
+            ('abc', 'def'): 'abc_def',
+            ('def', 'abc'): 'abc_def',
+            ('one', 'two', 'three'): 'one_three_two',
+        }
+        for args, result in expected.items():
+            self.assertEqual(result, idlutils.index_name(*args))
+
+        self.assertRaises(AssertionError, idlutils.index_name)

ovsdbapp/tests/unit/backend/test_ovs_idl.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import collections
 from unittest import mock
 
 from ovsdbapp.backend import ovs_idl
@@ -23,7 +24,9 @@ class FakeRow(object):
 
 
 class FakeTable(object):
-    rows = {'fake-id-1': FakeRow(uuid='fake-id-1', name='Fake1')}
+    rows = collections.UserDict({'fake-id-1':
+                                 FakeRow(uuid='fake-id-1', name='Fake1')})
+    rows.indexes = {}
     indexes = []
 
 
@@ -35,6 +38,9 @@ class FakeBackend(ovs_idl.Backend):
     def start_connection(self, connection):
         pass
 
+    def autocreate_indices(self):
+        pass
+
 
 class TestBackendOvsIdl(base.TestCase):
     def setUp(self):

ovsdbapp/tests/unit/schema/open_vswitch/test_impl_idl.py

@@ -48,5 +48,5 @@ class TestOvsdbIdl(base.TestCase):
 
     def test_init_session(self):
         conn = mock.MagicMock()
-        backend = impl_idl.OvsdbIdl(conn, start=False)
-        self.assertIsNone(backend.ovsdb_connection)
+        impl_idl.OvsdbIdl(conn, start=False)
+        conn.start_connection.assert_not_called()