Fix race condition in initial connection. 50/20050/5
authorEd Warnicke <hagbard@gmail.com>
Mon, 11 May 2015 18:56:09 +0000 (11:56 -0700)
committerEd Warnicke <hagbard@gmail.com>
Mon, 11 May 2015 20:11:49 +0000 (13:11 -0700)
We must register the OvsdbConnectionCreateCommand prior
to registerCallback or we will get race conditions.

*Before* any bridge information can be successfully written
to the operational datastore, the OvsdbNode *must* be written.
The txInvoker guarantees writes occur in the order they were
provided to *it*.

If we have:

  txInvoker.invoke(new OvsdbNodeCreateCommand(key, null,null));
  registerCallBack();

We can guarantee that, and thus guarantee we will not have race conditions.

However, if we reverse that:

  registerCallBack();
  txInvoker.invoke(new OvsdbNodeCreateCommand(key, null,null));

We are guarenteed to have race conditions, as the results of the original
monitor dump get written to the datastore before the OvsdbNode gets written.

Please note: ovsdb is actively broken out of the box due to this race
conditions, so this must be corrected ASAP.

Change-Id: Id71221f7665ece5ec4b2eb9a0b39465618bada01
Signed-off-by: Ed Warnicke <hagbard@gmail.com>
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionInstance.java
southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/OvsdbConnectionManager.java

index 4c5548879056daf81d4acec9eb580ece8e153c25..757ebd02479ff643a628e744b7bd9735a6ee7e77 100644 (file)
@@ -35,6 +35,7 @@ import org.opendaylight.ovsdb.lib.schema.typed.TypedBaseTable;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.TransactCommand;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.TransactInvoker;
 import org.opendaylight.ovsdb.southbound.ovsdb.transact.TransactInvokerImpl;
+import org.opendaylight.ovsdb.southbound.transactions.md.OvsdbNodeCreateCommand;
 import org.opendaylight.ovsdb.southbound.transactions.md.TransactionInvoker;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.ovsdb.node.attributes.ConnectionInfo;
 import org.slf4j.Logger;
@@ -50,12 +51,13 @@ public class OvsdbConnectionInstance implements OvsdbClient {
     private TransactionInvoker txInvoker;
     private Map<DatabaseSchema,TransactInvoker> transactInvokers = new HashMap<DatabaseSchema,TransactInvoker>();
     private MonitorCallBack callback;
+    private ConnectionInfo key;
 
     OvsdbConnectionInstance(ConnectionInfo key,OvsdbClient client,TransactionInvoker txInvoker) {
         this.connectionInfo = key;
         this.client = client;
         this.txInvoker = txInvoker;
-        registerCallBack();
+        this.key = key;
     }
 
     public void transact(TransactCommand command) {
@@ -64,25 +66,28 @@ public class OvsdbConnectionInstance implements OvsdbClient {
         }
     }
 
-    private void registerCallBack() {
-        this.callback = new OvsdbMonitorCallback(connectionInfo,txInvoker);
-        try {
-            List<String> databases = getDatabases().get();
-            if (databases != null) {
-                for (String database : databases) {
-                    DatabaseSchema dbSchema = getSchema(database).get();
-                    if (dbSchema != null) {
-                        transactInvokers.put(dbSchema, new TransactInvokerImpl(this,dbSchema));
-                        monitorAllTables(database, dbSchema);
-                    } else {
-                        LOG.warn("No schema reported for database {} for key {}",database,connectionInfo);
+    public void init() {
+        if ( this.callback == null) {
+            txInvoker.invoke(new OvsdbNodeCreateCommand(key, null,null));
+            this.callback = new OvsdbMonitorCallback(connectionInfo,txInvoker);
+            try {
+                List<String> databases = getDatabases().get();
+                if (databases != null) {
+                    for (String database : databases) {
+                        DatabaseSchema dbSchema = getSchema(database).get();
+                        if (dbSchema != null) {
+                            transactInvokers.put(dbSchema, new TransactInvokerImpl(this,dbSchema));
+                            monitorAllTables(database, dbSchema);
+                        } else {
+                            LOG.warn("No schema reported for database {} for key {}",database,connectionInfo);
+                        }
                     }
+                } else {
+                    LOG.warn("No databases reported from {}",connectionInfo);
                 }
-            } else {
-                LOG.warn("No databases reported from {}",connectionInfo);
+            } catch (InterruptedException | ExecutionException e) {
+                LOG.warn("Exception attempting to initialize {}: {}",connectionInfo,e);
             }
-        } catch (InterruptedException | ExecutionException e) {
-            LOG.warn("Exception attempting to initialize {}: {}",connectionInfo,e);
         }
     }
 
index dbb523627d42f2d20af7a557307ae1ece9061515..fc8361f72feee278e8ea54e2f6c9913d6c9d1824 100644 (file)
@@ -19,7 +19,6 @@ import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.ovsdb.lib.OvsdbClient;
 import org.opendaylight.ovsdb.lib.OvsdbConnectionListener;
 import org.opendaylight.ovsdb.lib.impl.OvsdbConnectionService;
-import org.opendaylight.ovsdb.southbound.transactions.md.OvsdbNodeCreateCommand;
 import org.opendaylight.ovsdb.southbound.transactions.md.OvsdbNodeRemoveCommand;
 import org.opendaylight.ovsdb.southbound.transactions.md.TransactionInvoker;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.ovsdb.rev150105.OvsdbBridgeAttributes;
@@ -55,7 +54,7 @@ public class OvsdbConnectionManager implements OvsdbConnectionListener, AutoClos
         ConnectionInfo key = SouthboundMapper.createConnectionInfo(externalClient);
         OvsdbConnectionInstance client = new OvsdbConnectionInstance(key,externalClient,txInvoker);
         putConnectionInstance(key, client);
-        txInvoker.invoke(new OvsdbNodeCreateCommand(key, null,null));
+        client.init();
     }
 
     @Override