ovsdb: Force strong references to non-root tables to be persistent.
authorBen Pfaff <blp@nicira.com>
Thu, 31 Mar 2011 23:43:43 +0000 (16:43 -0700)
committerBen Pfaff <blp@nicira.com>
Thu, 31 Mar 2011 23:43:43 +0000 (16:43 -0700)
When a strong reference to a non-root table is ephemeral, the database log
can contain inconsistencies.  In particular, if the column in question is
the only reference to a row, then the row will be created in one logged
transaction but the reference to it will not be logged (because it is
ephemeral).  Thus, any later occurrence of the row later in the log (to
modify it, to delete it, or just to reference it) will yield a transaction
error and reading the database will abort at that point.

This commit fixes the problem by forcing any column with a strong reference
to a non-root table to be persistent.

The change to ovsdb_schema_from_json() looks bigger than it really is: it
just swaps the order of two operations on the schema and updates their
comments.  Similarly for the update to ovs.db.DbSchema.__init__().

Bug #5144.
Reported-by: Sujatha Sumanth <ssumanth@nicira.com>
Bug #5149.
Reported-by: Ram Jothikumar <rjothikumar@nicira.com>
ovsdb/SPECS
ovsdb/ovsdb.c
python/ovs/db/schema.py
tests/ovsdb-schema.at

index 3a9af9f39a7acd91ca67e79a38926aab680c2262..d413c284510f66d29ad77b2860ba7a19bc63a87e 100644 (file)
@@ -184,12 +184,18 @@ is represented by <database-schema>, as described below.
         "ephemeral": <boolean>                    optional
         "mutable": <boolean>                      optional
 
-    The "type" specifies the type of data stored in this column.  If
-    "ephemeral" is specified as true, then this column's values are
+    The "type" specifies the type of data stored in this column.
+
+    If "ephemeral" is specified as true, then this column's values are
     not guaranteed to be durable; they may be lost when the database
-    restarts.  If "mutable" is specified as false, then this column's
-    values may not be modified after they are initially set with the
-    "insert" operation.
+    restarts.  A column whose type (either key or value) is a strong
+    reference to a table that is not part of the root set is always
+    durable, regardless of this value.  (Otherwise, restarting the
+    database could lose entire rows.)
+
+    If "mutable" is specified as false, then this column's values may
+    not be modified after they are initially set with the "insert"
+    operation.
 
 <type>
 
index d4a27d40ea3491c87305aa4e1892f29527895951..abe88e9f9fb86df34ac13025d716fb91c0a00460 100644 (file)
@@ -103,20 +103,36 @@ ovsdb_schema_from_file(const char *file_name, struct ovsdb_schema **schemap)
 }
 
 static struct ovsdb_error * WARN_UNUSED_RESULT
-ovsdb_schema_check_ref_table(const struct ovsdb_column *column,
+ovsdb_schema_check_ref_table(struct ovsdb_column *column,
                              const struct shash *tables,
                              const struct ovsdb_base_type *base,
                              const char *base_name)
 {
-    if (base->type == OVSDB_TYPE_UUID && base->u.uuid.refTableName
-        && !shash_find(tables, base->u.uuid.refTableName)) {
+    struct ovsdb_table_schema *refTable;
+
+    if (base->type != OVSDB_TYPE_UUID || !base->u.uuid.refTableName) {
+        return NULL;
+    }
+
+    refTable = shash_find_data(tables, base->u.uuid.refTableName);
+    if (!refTable) {
         return ovsdb_syntax_error(NULL, NULL,
                                   "column %s %s refers to undefined table %s",
                                   column->name, base_name,
                                   base->u.uuid.refTableName);
-    } else {
-        return NULL;
     }
+
+    if (ovsdb_base_type_is_strong_ref(base) && !refTable->is_root) {
+        /* We cannot allow a strong reference to a non-root table to be
+         * ephemeral: if it is the only reference to a row, then replaying the
+         * database log from disk will cause the referenced row to be deleted,
+         * even though it did exist in memory.  If there are references to that
+         * row later in the log (to modify it, to delete it, or just to point
+         * to it), then this will yield a transaction error. */
+        column->persistent = true;
+    }
+
+    return NULL;
 }
 
 static bool
@@ -198,7 +214,23 @@ ovsdb_schema_from_json(struct json *json, struct ovsdb_schema **schemap)
         shash_add(&schema->tables, table->name, table);
     }
 
-    /* Validate that all refTables refer to the names of tables that exist. */
+    /* "isRoot" was not part of the original schema definition.  Before it was
+     * added, there was no support for garbage collection.  So, for backward
+     * compatibility, if the root set is empty then assume that every table is
+     * in the root set. */
+    if (root_set_size(schema) == 0) {
+        SHASH_FOR_EACH (node, &schema->tables) {
+            struct ovsdb_table_schema *table = node->data;
+
+            table->is_root = true;
+        }
+    }
+
+    /* Validate that all refTables refer to the names of tables that exist.
+     *
+     * Also force certain columns to be persistent, as explained in
+     * ovsdb_schema_check_ref_table().  This requires 'is_root' to be known, so
+     * this must follow the loop updating 'is_root' above. */
     SHASH_FOR_EACH (node, &schema->tables) {
         struct ovsdb_table_schema *table = node->data;
         struct shash_node *node2;
@@ -220,18 +252,6 @@ ovsdb_schema_from_json(struct json *json, struct ovsdb_schema **schemap)
         }
     }
 
-    /* "isRoot" was not part of the original schema definition.  Before it was
-     * added, there was no support for garbage collection.  So, for backward
-     * compatibility, if the root set is empty then assume that every table is
-     * in the root set. */
-    if (root_set_size(schema) == 0) {
-        SHASH_FOR_EACH (node, &schema->tables) {
-            struct ovsdb_table_schema *table = node->data;
-
-            table->is_root = true;
-        }
-    }
-
     *schemap = schema;
     return 0;
 }
index e0e0daf5ec60b7bd093c4da8247676cbeadf2fec..9883adc2d69262d926cae5374489e6502239bb69 100644 (file)
@@ -27,13 +27,6 @@ class DbSchema(object):
         self.version = version
         self.tables = tables
 
-        # Validate that all ref_tables refer to the names of tables
-        # that exist.
-        for table in self.tables.itervalues():
-            for column in table.columns.itervalues():
-                self.__check_ref_table(column, column.type.key, "key")
-                self.__check_ref_table(column, column.type.value, "value")
-
         # "isRoot" was not part of the original schema definition.  Before it
         # was added, there was no support for garbage collection.  So, for
         # backward compatibility, if the root set is empty then assume that
@@ -42,6 +35,17 @@ class DbSchema(object):
             for table in self.tables.itervalues():
                 table.is_root = True
 
+        # Validate that all ref_tables refer to the names of tables
+        # that exist.
+        #
+        # Also force certain columns to be persistent, as explained in
+        # __check_ref_table().  This requires 'is_root' to be known, so this
+        # must follow the loop updating 'is_root' above.
+        for table in self.tables.itervalues():
+            for column in table.columns.itervalues():
+                self.__check_ref_table(column, column.type.key, "key")
+                self.__check_ref_table(column, column.type.value, "value")
+
     def __root_set_size(self):
         """Returns the number of tables in the schema's root set."""
         n_root = 0
@@ -91,12 +95,25 @@ class DbSchema(object):
         return json
 
     def __check_ref_table(self, column, base, base_name):
-        if (base and base.type == types.UuidType and base.ref_table and
-            base.ref_table not in self.tables):
+        if not base or base.type != types.UuidType or not base.ref_table:
+            return
+
+        ref_table = self.tables.get(base.ref_table)
+        if not ref_table:
             raise error.Error("column %s %s refers to undefined table %s"
                               % (column.name, base_name, base.ref_table),
                               tag="syntax error")
 
+        if base.is_strong_ref() and not ref_table.is_root:
+            # We cannot allow a strong reference to a non-root table to be
+            # ephemeral: if it is the only reference to a row, then replaying
+            # the database log from disk will cause the referenced row to be
+            # deleted, even though it did exist in memory.  If there are
+            # references to that row later in the log (to modify it, to delete
+            # it, or just to point to it), then this will yield a transaction
+            # error.
+            column.persistent = True
+
 class IdlSchema(DbSchema):
     def __init__(self, name, version, tables, idlPrefix, idlHeader):
         DbSchema.__init__(self, name, version, tables)
index d129093a216d1641b541d736196dbc4196599df8..00da808391d64c0a77c16868078b14eb6f940bdd 100644 (file)
@@ -23,6 +23,38 @@ OVSDB_CHECK_POSITIVE_CPY([schema with valid refTables],
                     "type": "uuid",
                     "refTable": "a"}}}}}}}']],
   [[{"name":"mydb","tables":{"a":{"columns":{"map":{"type":{"key":{"refTable":"b","type":"uuid"},"value":{"refTable":"a","type":"uuid"}}}}},"b":{"columns":{"aRef":{"type":{"key":{"refTable":"a","type":"uuid"}}}}}},"version":"4.2.1"}]])
+
+dnl Ephemeral strong references to root set tables are OK.
+dnl Ephemeral strong references to non-root set tables are forced to be
+dnl persistent.
+OVSDB_CHECK_POSITIVE_CPY([schema with ephemeral strong references],
+  [[parse-schema \
+      '{"name": "mydb",
+        "version": "4.2.1",
+        "tables": {
+          "a": {
+            "columns": {
+              "x": {
+                "type": {
+                  "key": {
+                    "type": "uuid",
+                    "refTable": "b"}},
+                "ephemeral": true},
+              "y": {
+                "type": {
+                  "key": {
+                    "type": "uuid",
+                    "refTable": "a"}},
+                "ephemeral": true}}},
+          "b": {
+            "columns": {
+              "aRef": {
+                "type": {
+                  "key": {
+                    "type": "uuid",
+                    "refTable": "a"}}}},
+              "isRoot": true}}}']],
+  [[{"name":"mydb","tables":{"a":{"columns":{"x":{"ephemeral":true,"type":{"key":{"refTable":"b","type":"uuid"}}},"y":{"type":{"key":{"refTable":"a","type":"uuid"}}}}},"b":{"columns":{"aRef":{"type":{"key":{"refTable":"a","type":"uuid"}}}},"isRoot":true}},"version":"4.2.1"}]])
      
 dnl Schemas without version numbers are accepted for backward
 dnl compatibility, but this is a deprecated feature.