ovs-vsctl: Try connecting only once for active connections by default.

Until now, ovs-vsctl has kept trying to the database server until it
succeeded or the timeout expired (if one was specified with --timeout).
This meant that if ovsdb-server wasn't running, then ovs-vsctl would hang.
The result was that almost every ovs-vsctl invocation in scripts specified
a timeout on the off-chance that the database server might not be running.
But it's difficult to choose a good timeout.  A timeout that is too short
can cause spurious failures.  A timeout that is too long causes long delays
if the server really isn't running.

This commit should alleviate this problem.  It changes ovs-vsctl's behavior
so that, if it fails to connect to the server, it exits unsuccessfully.
This makes --timeout obsolete for the purpose of avoiding a hang if the
database server isn't running.  (--timeout is still useful to avoid a hang
if ovsdb-server is running but ovs-vswitchd is not, for ovs-vsctl commands
that modify the database.  --no-wait also avoids that issue.)

Bug #2393.
Bug #15594.
Reported-by: Jeff Merrick <jmerrick@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

5 files changed, 49 insertions, 10 deletions

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 50073b6d7..1ea5398b4 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -744,6 +744,7 @@ struct jsonrpc_session {
     struct jsonrpc *rpc;
     struct stream *stream;
     struct pstream *pstream;
+    int last_error;
     unsigned int seqno;
     uint8_t dscp;
 };
@@ -752,14 +753,18 @@ struct jsonrpc_session {
  * acceptable to stream_open() or pstream_open().
  *
  * If 'name' is an active connection method, e.g. "tcp:127.1.2.3", the new
- * jsonrpc_session connects and reconnects, with back-off, to 'name'.
+ * jsonrpc_session connects to 'name'.  If 'retry' is true, then the new
+ * session connects and reconnects to 'name', with backoff.  If 'retry' is
+ * false, the new session will only try to connect once and after a connection
+ * failure or a disconnection jsonrpc_session_is_alive() will return false for
+ * the new session.
  *
  * If 'name' is a passive connection method, e.g. "ptcp:", the new
  * jsonrpc_session listens for connections to 'name'.  It maintains at most one
  * connection at any given time.  Any new connection causes the previous one
  * (if any) to be dropped. */
 struct jsonrpc_session *
-jsonrpc_session_open(const char *name)
+jsonrpc_session_open(const char *name, bool retry)
 {
     struct jsonrpc_session *s;
 
@@ -772,9 +777,13 @@ jsonrpc_session_open(const char *name)
     s->pstream = NULL;
     s->seqno = 0;
     s->dscp = 0;
+    s->last_error = 0;
 
     if (!pstream_verify_name(name)) {
         reconnect_set_passive(s->reconnect, true, time_msec());
+    } else if (!retry) {
+        reconnect_set_max_tries(s->reconnect, 1);
+        reconnect_set_backoff(s->reconnect, INT_MAX, INT_MAX);
     }
 
     if (!stream_or_pstream_needs_probes(name)) {
@@ -847,6 +856,8 @@ jsonrpc_session_connect(struct jsonrpc_session *s)
         error = jsonrpc_stream_open(name, &s->stream, s->dscp);
         if (!error) {
             reconnect_connecting(s->reconnect, time_msec());
+        } else {
+            s->last_error = error;
         }
     } else {
         error = s->pstream ? 0 : jsonrpc_pstream_open(name, &s->pstream,
@@ -908,6 +919,7 @@ jsonrpc_session_run(struct jsonrpc_session *s)
         if (error) {
             reconnect_disconnected(s->reconnect, time_msec(), error);
             jsonrpc_session_disconnect(s);
+            s->last_error = error;
         }
     } else if (s->stream) {
         int error;
@@ -1061,6 +1073,12 @@ jsonrpc_session_get_status(const struct jsonrpc_session *s)
     return s && s->rpc ? jsonrpc_get_status(s->rpc) : 0;
 }
 
+int
+jsonrpc_session_get_last_error(const struct jsonrpc_session *s)
+{
+    return s->last_error;
+}
+
 void
 jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *s,
                                     struct reconnect_stats *stats)
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index b5acf8939..0ae205d32 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -98,7 +98,7 @@ struct json *jsonrpc_msg_to_json(struct jsonrpc_msg *);
 
 /* A JSON-RPC session with reconnection. */
 
-struct jsonrpc_session *jsonrpc_session_open(const char *name);
+struct jsonrpc_session *jsonrpc_session_open(const char *name, bool retry);
 struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *,
                                                         uint8_t);
 void jsonrpc_session_close(struct jsonrpc_session *);
@@ -117,6 +117,7 @@ bool jsonrpc_session_is_alive(const struct jsonrpc_session *);
 bool jsonrpc_session_is_connected(const struct jsonrpc_session *);
 unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *);
 int jsonrpc_session_get_status(const struct jsonrpc_session *);
+int jsonrpc_session_get_last_error(const struct jsonrpc_session *);
 void jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *,
                                          struct reconnect_stats *);
 
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 0c644a560..116aa86c4 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -157,6 +157,9 @@ static void ovsdb_idl_parse_lock_notify(struct ovsdb_idl *,
  * 'class'.  (Ordinarily 'class' is compiled from an OVSDB schema automatically
  * by ovsdb-idlc.)
  *
+ * Passes 'retry' to jsonrpc_session_open().  See that function for
+ * documentation.
+ *
  * If 'monitor_everything_by_default' is true, then everything in the remote
  * database will be replicated by default.  ovsdb_idl_omit() and
  * ovsdb_idl_omit_alert() may be used to selectively drop some columns from
@@ -168,7 +171,7 @@ static void ovsdb_idl_parse_lock_notify(struct ovsdb_idl *,
  */
 struct ovsdb_idl *
 ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
-                 bool monitor_everything_by_default)
+                 bool monitor_everything_by_default, bool retry)
 {
     struct ovsdb_idl *idl;
     uint8_t default_mode;
@@ -180,7 +183,7 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
 
     idl = xzalloc(sizeof *idl);
     idl->class = class;
-    idl->session = jsonrpc_session_open(remote);
+    idl->session = jsonrpc_session_open(remote, retry);
     shash_init(&idl->table_by_name);
     idl->tables = xmalloc(class->n_tables * sizeof *idl->tables);
     for (i = 0; i < class->n_tables; i++) {
@@ -412,6 +415,18 @@ ovsdb_idl_verify_write_only(struct ovsdb_idl *idl)
 {
     idl->verify_write_only = true;
 }
+
+bool
+ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
+{
+    return jsonrpc_session_is_alive(idl->session);
+}
+
+int
+ovsdb_idl_get_last_error(const struct ovsdb_idl *idl)
+{
+    return jsonrpc_session_get_last_error(idl->session);
+}
 
 static unsigned char *
 ovsdb_idl_get_mode(struct ovsdb_idl *idl,
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index b428501fa..6b5e198b2 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -44,7 +44,8 @@ struct uuid;
 
 struct ovsdb_idl *ovsdb_idl_create(const char *remote,
                                    const struct ovsdb_idl_class *,
-                                   bool monitor_everything_by_default);
+                                   bool monitor_everything_by_default,
+                                   bool retry);
 void ovsdb_idl_destroy(struct ovsdb_idl *);
 
 void ovsdb_idl_run(struct ovsdb_idl *);
@@ -58,6 +59,9 @@ unsigned int ovsdb_idl_get_seqno(const struct ovsdb_idl *);
 bool ovsdb_idl_has_ever_connected(const struct ovsdb_idl *);
 void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
 void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
+
+bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
+int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
 
 /* Choosing columns and tables to replicate. */
 
diff --git a/lib/reconnect.c b/lib/reconnect.c
index 40cc7fcef..b914ef687 100644
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -207,7 +207,8 @@ reconnect_get_max_tries(struct reconnect *fsm)
 
 /* Configures the backoff parameters for 'fsm'.  'min_backoff' is the minimum
  * number of milliseconds, and 'max_backoff' is the maximum, between connection
- * attempts.
+ * attempts.  The current backoff is also the duration that 'fsm' is willing to
+ * wait for a given connection to succeed or fail.
  *
  * 'min_backoff' must be at least 1000, and 'max_backoff' must be greater than
  * or equal to 'min_backoff'.