ovsdb-server: Replace in-memory DB contents at raft install_snapshot.

Every time a follower has to install a snapshot received from the
leader, it should also replace the data in memory. Right now this only
happens when snapshots are installed that also change the schema.

This can lead to inconsistent DB data on follower nodes and the snapshot
may fail to get applied.

Fixes: bda1f6b60588 ("ovsdb-server: Don't disconnect clients after raft install_snapshot.")
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index ef4e996df22e59bb4bb94cc329363328b1f71f08..fd7891a7294e9bd652fb5d6c8c3f67100cea5961 100644 (file)
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -543,13 +543,14 @@ parse_txn(struct server_config *config, struct db *db,
           const struct ovsdb_schema *schema, const struct json *txn_json,
           const struct uuid *txnid)
 {
-    if (schema && (!db->db->schema || strcmp(schema->version,
-                                             db->db->schema->version))) {
+    if (schema) {
         /* We're replacing the schema (and the data).  Destroy the database
          * (first grabbing its storage), then replace it with the new schema.
          * The transaction must also include the replacement data.
          *
-         * Only clustered database schema changes go through this path. */
+         * Only clustered database schema changes and snapshot installs
+         * go through this path.
+         */
         ovs_assert(txn_json);
         ovs_assert(ovsdb_storage_is_clustered(db->db->storage));
 
@@ -559,11 +560,15 @@ parse_txn(struct server_config *config, struct db *db,
             return error;
         }
 
-        ovsdb_jsonrpc_server_reconnect(
-            config->jsonrpc, false,
-            (db->db->schema
-             ? xasprintf("database %s schema changed", db->db->name)
-             : xasprintf("database %s connected to storage", db->db->name)));
+        if (!db->db->schema ||
+            strcmp(schema->version, db->db->schema->version)) {
+            ovsdb_jsonrpc_server_reconnect(
+                config->jsonrpc, false,
+                (db->db->schema
+                ? xasprintf("database %s schema changed", db->db->name)
+                : xasprintf("database %s connected to storage",
+                            db->db->name)));
+        }
 
         ovsdb_replace(db->db, ovsdb_create(ovsdb_schema_clone(schema), NULL));
 

diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
index e02b975bc5487f33a3ee3b7e3a81af8dd395fd8d..e04755ea055ff669e662549093ff17e6a6cae9ba 100644 (file)
--- a/tests/idltest.ovsschema
+++ b/tests/idltest.ovsschema
@@ -54,6 +54,15 @@
       },
       "isRoot" : true
     },
+    "indexed": {
+      "columns": {
+        "i": {
+          "type": "integer"
+        }
+      },
+      "indexes": [["i"]],
+      "isRoot" : true
+    },
     "simple": {
       "columns": {
         "b": {

diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index 9714545151fae52cacf2d311e1788d32faf9cfce..e0758e954cc337e9a7305b252736ecac617182f2 100644 (file)
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -332,13 +332,29 @@ for i in `seq $n`; do
     AT_CHECK([ovsdb_client_wait unix:s$i.ovsdb $schema_name connected])
 done
 
+AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest",
+      {"op": "insert",
+       "table": "indexed",
+       "row": {"i": 0}}]]'], [0], [ignore], [ignore])
+
 # Kill one follower (s2) and write some data to cluster, so that the follower is falling behind
 printf "\ns2: stopping\n"
 OVS_APP_EXIT_AND_WAIT_BY_TARGET([`pwd`/s2], [s2.pid])
 
+# Delete "i":0 and readd it to get a different UUID for it.
+AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest",
+      {"op": "delete",
+       "table": "indexed",
+       "where": [["i", "==", 0]]}]]'], [0], [ignore], [ignore])
+
 AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest",
       {"op": "insert",
-       "table": "simple",
+       "table": "indexed",
+       "row": {"i": 0}}]]'], [0], [ignore], [ignore])
+
+AT_CHECK([ovsdb-client transact unix:s1.ovsdb '[["idltest",
+      {"op": "insert",
+       "table": "indexed",
        "row": {"i": 1}}]]'], [0], [ignore], [ignore])
 
 # Compact leader online to generate snapshot
@@ -355,8 +371,18 @@ AT_CHECK([ovsdb_client_wait unix:s2.ovsdb $schema_name connected])
 # succeed.
 AT_CHECK([ovsdb-client transact unix:s2.ovsdb '[["idltest",
       {"op": "insert",
-       "table": "simple",
-       "row": {"i": 1}}]]'], [0], [ignore], [ignore])
+       "table": "indexed",
+       "row": {"i": 2}}]]'], [0], [ignore], [ignore])
+
+# The snapshot should overwrite the in-memory contents of the DB on S2
+# without generating any constraint violations. All tree records (0, 1, 2)
+# should be in the DB at this point.
+AT_CHECK([ovsdb-client --no-headings dump unix:s2.ovsdb idltest indexed | uuidfilt | sort -k 2], [0], [dnl
+<0> 0
+<1> 1
+<2> 2
+indexed table
+])
 
 for i in `seq $n`; do
     OVS_APP_EXIT_AND_WAIT_BY_TARGET([`pwd`/s$i], [s$i.pid])

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 4efed88e48aafc2cbe54eabeb48f889befbe009a..789ae23a97b42012149e084923ee04fb4c9087fb 100644 (file)
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -954,6 +954,7 @@ AT_CHECK([sort stdout | uuidfilt], [0],
 
 # Check that ovsdb-idl figured out that table link2 and column l2 are missing.
 AT_CHECK([grep ovsdb_idl stderr | sort], [0], [dnl
+test-ovsdb|ovsdb_idl|idltest database lacks indexed table (database needs upgrade?)
 test-ovsdb|ovsdb_idl|idltest database lacks link2 table (database needs upgrade?)
 test-ovsdb|ovsdb_idl|idltest database lacks simple5 table (database needs upgrade?)
 test-ovsdb|ovsdb_idl|idltest database lacks singleton table (database needs upgrade?)