Revert "jsonrpc-server: Disconnect connections that queue too much data."

This reverts commit 60533a405b2eb77214b767767fe143c8645f82d5.

Connections that queue up too much data, because they are monitoring a
table that is changing quickly and failing to keep up with the updates,
cause problems with buffer management.  Since commit 60533a405b2e
(jsonrpc-server: Disconnect connections that queue too much data.),
ovsdb-server has dealt with them by disconnecting the connection and
letting them start up again with a fresh copy of the database.  However,
this is not ideal because of situations where disconnection happens
repeatedly.  For example:

     - A manager toggles a column back and forth between two or more values
       quickly (in which case the data transmitted over the monitoring
       connections always increases quickly, without bound).

     - A manager repeatedly extends the contents of some column in some row
       (in which case the data transmitted over the monitoring connection
       grows with O(n**2) in the length of the string).

A better way to deal with this problem is to combine updates when they are
sent to the monitoring connection, if that connection is not keeping up.
In both the above cases, this reduces the data that must be sent to a
manageable amount.  An upcoming patch implements this new way.  This commit
reverts part of the previous solution that disconnects backlogged
connections, since it is no longer useful.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 4758442..a81648b 100644 (file)
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -82,8 +82,6 @@ static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
     struct json_array *params,
     const struct json *request_id);
 static void ovsdb_jsonrpc_monitor_remove_all(struct ovsdb_jsonrpc_session *);
-static size_t ovsdb_jsonrpc_monitor_json_length_all(
-    struct ovsdb_jsonrpc_session *);
 \f
 /* JSON-RPC database server. */
 
@@ -364,8 +362,6 @@ struct ovsdb_jsonrpc_session {
     struct list node;           /* Element in remote's sessions list. */
     struct ovsdb_session up;
     struct ovsdb_jsonrpc_remote *remote;
-    size_t backlog_threshold;   /* See ovsdb_jsonrpc_session_run(). */
-    size_t reply_backlog;
 
     /* Triggers. */
     struct hmap triggers;       /* Hmap of "struct ovsdb_jsonrpc_trigger"s. */
@@ -402,8 +398,6 @@ ovsdb_jsonrpc_session_create(struct ovsdb_jsonrpc_remote *remote,
     list_push_back(&remote->sessions, &s->node);
     hmap_init(&s->triggers);
     hmap_init(&s->monitors);
-    s->reply_backlog = 0;
-    s->backlog_threshold = 1024 * 1024;
     s->js = js;
     s->js_seqno = jsonrpc_session_get_seqno(js);
 
@@ -432,8 +426,6 @@ ovsdb_jsonrpc_session_close(struct ovsdb_jsonrpc_session *s)
 static int
 ovsdb_jsonrpc_session_run(struct ovsdb_jsonrpc_session *s)
 {
-    size_t backlog;
-
     jsonrpc_session_run(s->js);
     if (s->js_seqno != jsonrpc_session_get_seqno(s->js)) {
         s->js_seqno = jsonrpc_session_get_seqno(s->js);
@@ -444,8 +436,7 @@ ovsdb_jsonrpc_session_run(struct ovsdb_jsonrpc_session *s)
 
     ovsdb_jsonrpc_trigger_complete_done(s);
 
-    backlog = jsonrpc_session_get_backlog(s->js);
-    if (!backlog) {
+    if (!jsonrpc_session_get_backlog(s->js)) {
         struct jsonrpc_msg *msg = jsonrpc_session_recv(s->js);
         if (msg) {
             if (msg->type == JSONRPC_REQUEST) {
@@ -460,39 +451,6 @@ ovsdb_jsonrpc_session_run(struct ovsdb_jsonrpc_session *s)
                 jsonrpc_msg_destroy(msg);
             }
         }
-        s->reply_backlog = jsonrpc_session_get_backlog(s->js);
-    } else if (backlog > s->reply_backlog + s->backlog_threshold) {
-        /* We have a lot of data queued to send to the client.  The data is
-         * likely to be mostly monitor updates.  It is unlikely that the
-         * monitor updates are due to transactions by 's', because we will not
-         * let 's' make any more transactions until it drains its backlog to 0
-         * (see previous 'if' case).  So the monitor updates are probably due
-         * to transactions made by database clients other than 's'.  We can't
-         * fix that by preventing 's' from executing more transactions.  We
-         * could fix it by preventing every client from executing transactions,
-         * but then one slow or hung client could prevent other clients from
-         * doing useful work.
-         *
-         * Our solution is to cap the maximum backlog to O(1) in the amount of
-         * data in the database.  If the backlog exceeds that amount, then we
-         * disconnect the client.  When it reconnects, it can fetch the entire
-         * contents of the database using less data than was previously
-         * backlogged. */
-        size_t monitor_length;
-
-        monitor_length = ovsdb_jsonrpc_monitor_json_length_all(s);
-        if (backlog > s->reply_backlog + monitor_length * 2) {
-            VLOG_INFO("%s: %"PRIuSIZE" bytes backlogged but a complete replica "
-                      "would only take %"PRIuSIZE" bytes, disconnecting",
-                      jsonrpc_session_get_name(s->js),
-                      backlog - s->reply_backlog, monitor_length);
-            jsonrpc_session_force_reconnect(s->js);
-        } else {
-            /* The backlog is not unreasonably big.  Only check again after it
-             * becomes much bigger. */
-            s->backlog_threshold = 2 * MAX(s->backlog_threshold * 2,
-                                           monitor_length);
-        }
     }
     return jsonrpc_session_is_alive(s->js) ? 0 : ETIMEDOUT;
 }
@@ -1096,8 +1054,6 @@ struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
 static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_replica *);
 static struct json *ovsdb_jsonrpc_monitor_get_initial(
     const struct ovsdb_jsonrpc_monitor *);
-static size_t ovsdb_jsonrpc_monitor_json_length(
-    const struct ovsdb_jsonrpc_monitor *);
 
 static bool
 parse_bool(struct ovsdb_parser *parser, const char *name, bool default_value)
@@ -1367,22 +1323,6 @@ ovsdb_jsonrpc_monitor_remove_all(struct ovsdb_jsonrpc_session *s)
     }
 }
 
-/* Returns an overestimate of the number of bytes of JSON data required to
- * report the current contents of the database over all the monitors currently
- * configured in 's'.  */
-static size_t
-ovsdb_jsonrpc_monitor_json_length_all(struct ovsdb_jsonrpc_session *s)
-{
-    struct ovsdb_jsonrpc_monitor *m;
-    size_t length;
-
-    length = 0;
-    HMAP_FOR_EACH (m, node, &s->monitors) {
-        length += ovsdb_jsonrpc_monitor_json_length(m);
-    }
-    return length;
-}
-
 static struct ovsdb_jsonrpc_monitor *
 ovsdb_jsonrpc_monitor_cast(struct ovsdb_replica *replica)
 {
@@ -1518,52 +1458,6 @@ ovsdb_jsonrpc_monitor_change_cb(const struct ovsdb_row *old,
     return true;
 }
 
-/* Returns an overestimate of the number of bytes of JSON data required to
- * report the current contents of the database over monitor 'm'. */
-static size_t
-ovsdb_jsonrpc_monitor_json_length(const struct ovsdb_jsonrpc_monitor *m)
-{
-    const struct shash_node *node;
-    size_t length;
-
-    /* Top-level overhead of monitor JSON. */
-    length = 256;
-
-    SHASH_FOR_EACH (node, &m->tables) {
-        const struct ovsdb_jsonrpc_monitor_table *mt = node->data;
-        const struct ovsdb_table *table = mt->table;
-        const struct ovsdb_row *row;
-        size_t i;
-
-        /* Per-table JSON overhead: "<table>":{...}. */
-        length += strlen(table->schema->name) + 32;
-
-        /* Per-row JSON overhead: ,"<uuid>":{"old":{...},"new":{...}} */
-        length += hmap_count(&table->rows) * (UUID_LEN + 32);
-
-        /* Per-row, per-column JSON overhead: ,"<column>": */
-        for (i = 0; i < mt->n_columns; i++) {
-            const struct ovsdb_jsonrpc_monitor_column *c = &mt->columns[i];
-            const struct ovsdb_column *column = c->column;
-
-            length += hmap_count(&table->rows) * (8 + strlen(column->name));
-        }
-
-        /* Data. */
-        HMAP_FOR_EACH (row, hmap_node, &table->rows) {
-            for (i = 0; i < mt->n_columns; i++) {
-                const struct ovsdb_jsonrpc_monitor_column *c = &mt->columns[i];
-                const struct ovsdb_column *column = c->column;
-
-                length += ovsdb_datum_json_length(&row->fields[column->index],
-                                                  &column->type);
-            }
-        }
-    }
-
-    return length;
-}
-
 static void
 ovsdb_jsonrpc_monitor_init_aux(struct ovsdb_jsonrpc_monitor_aux *aux,
                                const struct ovsdb_jsonrpc_monitor *m,