ovsdb-idl: Make selecting tables and columns to replicate more flexible.
authorBen Pfaff <blp@nicira.com>
Tue, 16 Nov 2010 17:14:52 +0000 (09:14 -0800)
committerBen Pfaff <blp@nicira.com>
Tue, 16 Nov 2010 17:14:52 +0000 (09:14 -0800)
Until now, by default the IDL replicated all tables and all columns in the
database, and a few functions made it possible to avoid replicating
selected columns.  This commit adds a mode in which nothing is replicated
by default and the client code is responsible for specifying each column
and table that it is interested in.  The following commit adds a user for
this mode.

lib/ovsdb-idl-provider.h
lib/ovsdb-idl.c
lib/ovsdb-idl.h
tests/test-ovsdb.c
utilities/ovs-vsctl.c
vswitchd/bridge.c
vswitchd/ovs-brcompatd.c

index 040a699964b38c0c9991517f67f4c0ac8ae3a155..87f62d64ce41214771c0a32174a8b2ede08c9807 100644 (file)
@@ -52,31 +52,10 @@ struct ovsdb_idl_table_class {
     size_t allocation_size;
 };
 
-enum ovsdb_idl_mode {
-    /* Client reads and may write this column and wants to be alerted upon
-     * updates to it.
-     *
-     * This is the default. */
-    OVSDB_IDL_MODE_RW,
-
-    /* Client may read and write this column, but doesn't care to be alerted
-     * when it is updated.
-     *
-     * This is useful for columns that a client treats as "write-only", that
-     * is, it updates them but doesn't want to get alerted about its own
-     * updates.  It also won't be alerted about other clients' updates, so this
-     * is suitable only for use by a client that "owns" a particular column. */
-    OVSDB_IDL_MODE_WO,
-
-    /* Client won't read or write this column at all.  The IDL code can't
-     * prevent reading the column, but writing will cause assertion
-     * failures. */
-    OVSDB_IDL_MODE_NONE
-};
-
 struct ovsdb_idl_table {
     const struct ovsdb_idl_table_class *class;
-    unsigned char *modes;    /* One of OVSDB_MODE_*, indexed by column. */
+    unsigned char *modes;    /* OVSDB_IDL_* bitmasks, indexed by column. */
+    bool need_table;         /* Monitor table even if no columns? */
     struct shash columns;    /* Contains "const struct ovsdb_idl_column *"s. */
     struct hmap rows;        /* Contains "struct ovsdb_idl_row"s. */
     struct ovsdb_idl *idl;   /* Containing idl. */
index 46cc51ee70e3ffb06e177996fe87253ff1d7bbbf..448f9c94424e35030f9d4794c7133c476db831b7 100644 (file)
@@ -66,7 +66,7 @@ struct ovsdb_idl {
     const struct ovsdb_idl_class *class;
     struct jsonrpc_session *session;
     struct shash table_by_name;
-    struct ovsdb_idl_table *tables;
+    struct ovsdb_idl_table *tables; /* Contains "struct ovsdb_idl_table *"s.*/
     struct json *monitor_request_id;
     unsigned int last_monitor_request_seqno;
     unsigned int change_seqno;
@@ -140,13 +140,29 @@ static bool ovsdb_idl_txn_process_reply(struct ovsdb_idl *,
  * form acceptable to jsonrpc_session_open().  The connection will maintain an
  * in-memory replica of the remote database whose schema is described by
  * 'class'.  (Ordinarily 'class' is compiled from an OVSDB schema automatically
- * by ovsdb-idlc.) */
+ * by ovsdb-idlc.)
+ *
+ * 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
+ * monitoring.
+ *
+ * If 'monitor_everything_by_default' is false, then no columns or tables will
+ * be replicated by default.  ovsdb_idl_add_column() and ovsdb_idl_add_table()
+ * must be used to choose some columns or tables to replicate.
+ */
 struct ovsdb_idl *
-ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class)
+ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
+                 bool monitor_everything_by_default)
 {
     struct ovsdb_idl *idl;
+    uint8_t default_mode;
     size_t i;
 
+    default_mode = (monitor_everything_by_default
+                    ? OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT
+                    : 0);
+
     idl = xzalloc(sizeof *idl);
     idl->class = class;
     idl->session = jsonrpc_session_open(remote);
@@ -160,7 +176,8 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class)
         shash_add_assert(&idl->table_by_name, tc->name, table);
         table->class = tc;
         table->modes = xmalloc(tc->n_columns);
-        memset(table->modes, OVSDB_IDL_MODE_RW, tc->n_columns);
+        memset(table->modes, default_mode, tc->n_columns);
+        table->need_table = false;
         shash_init(&table->columns);
         for (j = 0; j < tc->n_columns; j++) {
             const struct ovsdb_idl_column *column = &tc->columns[j];
@@ -359,64 +376,113 @@ ovsdb_idl_force_reconnect(struct ovsdb_idl *idl)
 {
     jsonrpc_session_force_reconnect(idl->session);
 }
-
-static void
-ovsdb_idl_set_mode(struct ovsdb_idl *idl,
-                   const struct ovsdb_idl_column *column,
-                   enum ovsdb_idl_mode mode)
+\f
+static unsigned char *
+ovsdb_idl_get_mode(struct ovsdb_idl *idl,
+                   const struct ovsdb_idl_column *column)
 {
     size_t i;
 
+    assert(!idl->change_seqno);
+
     for (i = 0; i < idl->class->n_tables; i++) {
         const struct ovsdb_idl_table *table = &idl->tables[i];
         const struct ovsdb_idl_table_class *tc = table->class;
 
         if (column >= tc->columns && column < &tc->columns[tc->n_columns]) {
-            unsigned char *modep = &table->modes[column - tc->columns];
-            assert(*modep == OVSDB_IDL_MODE_RW || *modep == mode);
-            *modep = mode;
-            return;
+            return &table->modes[column - tc->columns];
         }
     }
 
     NOT_REACHED();
 }
 
-/* By default, 'idl' replicates all of the columns in the remote database, and
- * ovsdb_idl_run() returns true upon a change to any column in the database.
- * Call this function to avoid alerting ovsdb_idl_run()'s caller upon changes
- * to 'column'.
+static void
+add_ref_table(struct ovsdb_idl *idl, const struct ovsdb_base_type *base)
+{
+    if (base->type == OVSDB_TYPE_UUID && base->u.uuid.refTableName) {
+        struct ovsdb_idl_table *table;
+
+        table = shash_find_data(&idl->table_by_name,
+                                base->u.uuid.refTableName);
+        if (table) {
+            table->need_table = true;
+        } else {
+            VLOG_WARN("%s IDL class missing referenced table %s",
+                      idl->class->database, base->u.uuid.refTableName);
+        }
+    }
+}
+
+/* Turns on OVSDB_IDL_MONITOR and OVSDB_IDL_ALERT for 'column' in 'idl'.  Also
+ * ensures that any tables referenced by 'column' will be replicated, even if
+ * no columns in that table are selected for replication (see
+ * ovsdb_idl_add_table() for more information).
  *
- * This is useful for columns that a client treats as "write-only", that is, it
- * updates them but doesn't want to get alerted about its own updates.  It also
- * won't be alerted about other clients' updates, so this is suitable only for
- * use by a client that "owns" a particular column.
+ * This function is only useful if 'monitor_everything_by_default' was false in
+ * the call to ovsdb_idl_create().  This function should be called between
+ * ovsdb_idl_create() and the first call to ovsdb_idl_run().
+ */
+void
+ovsdb_idl_add_column(struct ovsdb_idl *idl,
+                     const struct ovsdb_idl_column *column)
+{
+    *ovsdb_idl_get_mode(idl, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
+    add_ref_table(idl, &column->type.key);
+    add_ref_table(idl, &column->type.value);
+}
+
+/* Ensures that the table with class 'tc' will be replicated on 'idl' even if
+ * no columns are selected for replication.  This can be useful because it
+ * allows 'idl' to keep track of what rows in the table actually exist, which
+ * in turn allows columns that reference the table to have accurate contents.
+ * (The IDL presents the database with references to rows that do not exist
+ * removed.)
  *
- * The client must be careful not to retain pointers to data in 'column' across
- * calls to ovsdb_idl_run(), even when that function returns false, because
- * the client is not alerted to changes.
+ * This function is only useful if 'monitor_everything_by_default' was false in
+ * the call to ovsdb_idl_create().  This function should be called between
+ * ovsdb_idl_create() and the first call to ovsdb_idl_run().
+ */
+void
+ovsdb_idl_add_table(struct ovsdb_idl *idl,
+                    const struct ovsdb_idl_table_class *tc)
+{
+    size_t i;
+
+    for (i = 0; i < idl->class->n_tables; i++) {
+        struct ovsdb_idl_table *table = &idl->tables[i];
+
+        if (table->class == tc) {
+            table->need_table = true;
+            return;
+        }
+    }
+
+    NOT_REACHED();
+}
+
+/* Turns off OVSDB_IDL_ALERT for 'column' in 'idl'.
  *
- * This function should be called after ovsdb_idl_create(), but before the
- * first call to ovsdb_idl_run().  For any given column, this function may be
- * called or ovsdb_idl_omit() may be called, but not both. */
+ * This function should be called between ovsdb_idl_create() and the first call
+ * to ovsdb_idl_run().
+ */
 void
-ovsdb_idl_set_write_only(struct ovsdb_idl *idl,
-                         const struct ovsdb_idl_column *column)
+ovsdb_idl_omit_alert(struct ovsdb_idl *idl,
+                     const struct ovsdb_idl_column *column)
 {
-    ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_WO);
+    *ovsdb_idl_get_mode(idl, column) &= ~OVSDB_IDL_ALERT;
 }
 
-/* By default, 'idl' replicates all of the columns in the remote database.
- * Call this function to omit replicating 'column'.  This saves CPU time and
- * bandwidth to the database.
+/* Sets the mode for 'column' in 'idl' to 0.  See the big comment above
+ * OVSDB_IDL_MONITOR for details.
  *
- * This function should be called after ovsdb_idl_create(), but before the
- * first call to ovsdb_idl_run().  For any given column, this function may be
- * called or ovsdb_idl_set_write_only() may be called, but not both. */
+ * This function should be called between ovsdb_idl_create() and the first call
+ * to ovsdb_idl_run().
+ */
 void
 ovsdb_idl_omit(struct ovsdb_idl *idl, const struct ovsdb_idl_column *column)
 {
-    ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_NONE);
+    *ovsdb_idl_get_mode(idl, column) = 0;
 }
 \f
 static void
@@ -433,16 +499,22 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
         struct json *monitor_request, *columns;
         size_t j;
 
-        monitor_request = json_object_create();
-        columns = json_array_create_empty();
+        columns = table->need_table ? json_array_create_empty() : NULL;
         for (j = 0; j < tc->n_columns; j++) {
             const struct ovsdb_idl_column *column = &tc->columns[j];
-            if (table->modes[j] != OVSDB_IDL_MODE_NONE) {
+            if (table->modes[j] & OVSDB_IDL_MONITOR) {
+                if (!columns) {
+                    columns = json_array_create_empty();
+                }
                 json_array_add(columns, json_string_create(column->name));
             }
         }
-        json_object_put(monitor_request, "columns", columns);
-        json_object_put(monitor_requests, tc->name, monitor_request);
+
+        if (columns) {
+            monitor_request = json_object_create();
+            json_object_put(monitor_request, "columns", columns);
+            json_object_put(monitor_requests, tc->name, monitor_request);
+        }
     }
 
     json_destroy(idl->monitor_request_id);
@@ -642,7 +714,7 @@ ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
 
             if (!ovsdb_datum_equals(old, &datum, &column->type)) {
                 ovsdb_datum_swap(old, &datum);
-                if (table->modes[column_idx] == OVSDB_IDL_MODE_RW) {
+                if (table->modes[column_idx] & OVSDB_IDL_ALERT) {
                     changed = true;
                 }
             } else {
@@ -1563,7 +1635,8 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
 
     assert(row->new != NULL);
     assert(column_idx < class->n_columns);
-    assert(row->table->modes[column_idx] != OVSDB_IDL_MODE_NONE);
+    assert(row->old == NULL ||
+           row->table->modes[column_idx] & OVSDB_IDL_MONITOR);
 
     if (hmap_node_is_null(&row->txn_node)) {
         hmap_insert(&row->table->idl->txn->txn_rows, &row->txn_node,
index 9179e380eb1bb0f99d1dc2f6a423771548a2ef4c..302abd6a6be724e97a7338e3044aaa125b559a4c 100644 (file)
@@ -43,7 +43,8 @@ struct ovsdb_idl_table_class;
 struct uuid;
 
 struct ovsdb_idl *ovsdb_idl_create(const char *remote,
-                                   const struct ovsdb_idl_class *);
+                                   const struct ovsdb_idl_class *,
+                                   bool monitor_everything_by_default);
 void ovsdb_idl_destroy(struct ovsdb_idl *);
 
 bool ovsdb_idl_run(struct ovsdb_idl *);
@@ -52,10 +53,44 @@ void ovsdb_idl_wait(struct ovsdb_idl *);
 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 *);
+\f
+/* Choosing columns and tables to replicate. */
+
+/* Modes with which the IDL can monitor a column.
+ *
+ * If no bits are set, the column is not monitored at all.  Its value will
+ * always appear to the client to be the default value for its type.
+ *
+ * If OVSDB_IDL_MONITOR is set, then the column is replicated.  Its value will
+ * reflect the value in the database.  If OVSDB_IDL_ALERT is also set, then
+ * ovsdb_idl_run() will return "true", and the value returned by
+ * ovsdb_idl_get_seqno() will change, when the column's value changes.
+ *
+ * The possible mode combinations are:
+ *
+ *   - 0, for a column that a client doesn't care about.
+ *
+ *   - (OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT), for a column that a client wants
+ *     to track and possibly update.
+ *
+ *   - OVSDB_IDL_MONITOR, for columns that a client treats as "write-only",
+ *     that is, it updates them but doesn't want to get alerted about its own
+ *     updates.  It also won't be alerted about other clients' updates, so this
+ *     is suitable only for use by a client that "owns" a particular column.
+ *
+ *   - OVDSB_IDL_ALERT without OVSDB_IDL_MONITOR is not valid.
+ */
+#define OVSDB_IDL_MONITOR (1 << 0) /* Monitor this column? */
+#define OVSDB_IDL_ALERT   (1 << 1) /* Alert client when column updated? */
+
+void ovsdb_idl_add_column(struct ovsdb_idl *, const struct ovsdb_idl_column *);
+void ovsdb_idl_add_table(struct ovsdb_idl *,
+                         const struct ovsdb_idl_table_class *);
 
-void ovsdb_idl_set_write_only(struct ovsdb_idl *,
-                              const struct ovsdb_idl_column *);
 void ovsdb_idl_omit(struct ovsdb_idl *, const struct ovsdb_idl_column *);
+void ovsdb_idl_omit_alert(struct ovsdb_idl *, const struct ovsdb_idl_column *);
+\f
+/* Reading the database replica. */
 
 const struct ovsdb_idl_row *ovsdb_idl_get_row_for_uuid(
     const struct ovsdb_idl *, const struct ovsdb_idl_table_class *,
@@ -70,6 +105,8 @@ const struct ovsdb_datum *ovsdb_idl_get(const struct ovsdb_idl_row *,
                                         const struct ovsdb_idl_column *,
                                         enum ovsdb_atomic_type key_type,
                                         enum ovsdb_atomic_type value_type);
+\f
+/* Transactions. */
 
 enum ovsdb_idl_txn_status {
     TXN_UNCHANGED,              /* Transaction didn't include any changes. */
index 5a5a9083d4525b6e10e489f53e32a45b678e1870..aca68dcd359e53667242659ba841b4f104d83985 100644 (file)
@@ -1842,7 +1842,7 @@ do_idl(int argc, char *argv[])
 
     idltest_init();
 
-    idl = ovsdb_idl_create(argv[1], &idltest_idl_class);
+    idl = ovsdb_idl_create(argv[1], &idltest_idl_class, true);
     if (argc > 2) {
         struct stream *stream;
 
index b8e6470f41bd3f69930dc578bdcb645529e9551b..3326b2a42a10d13747705a26f3d5cfd35196dca3 100644 (file)
@@ -145,7 +145,7 @@ main(int argc, char *argv[])
     }
 
     /* Now execute the commands. */
-    idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class);
+    idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class, true);
     for (;;) {
         if (ovsdb_idl_run(idl)) {
             do_vsctl(args, commands, n_commands, idl);
index ff9ddfdf5dbf5b7c9e39cce97fdb7078bb7359aa..13e897fd5586061987b9e38b1417f9de1651c976 100644 (file)
@@ -274,10 +274,10 @@ void
 bridge_init(const char *remote)
 {
     /* Create connection to database. */
-    idl = ovsdb_idl_create(remote, &ovsrec_idl_class);
+    idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true);
 
-    ovsdb_idl_set_write_only(idl, &ovsrec_open_vswitch_col_cur_cfg);
-    ovsdb_idl_set_write_only(idl, &ovsrec_open_vswitch_col_statistics);
+    ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_cur_cfg);
+    ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_statistics);
     ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_external_ids);
 
     ovsdb_idl_omit(idl, &ovsrec_bridge_col_external_ids);
@@ -285,8 +285,8 @@ bridge_init(const char *remote)
     ovsdb_idl_omit(idl, &ovsrec_port_col_external_ids);
     ovsdb_idl_omit(idl, &ovsrec_port_col_fake_bridge);
 
-    ovsdb_idl_set_write_only(idl, &ovsrec_interface_col_ofport);
-    ovsdb_idl_set_write_only(idl, &ovsrec_interface_col_statistics);
+    ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_ofport);
+    ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_statistics);
     ovsdb_idl_omit(idl, &ovsrec_interface_col_external_ids);
 
     /* Register unixctl commands. */
index 6feeda054118247881459a0e5a7ed9c83f2aa787..b61dc10a0c5c2ba3df40c1c312d769fcb6700a00 100644 (file)
@@ -1324,7 +1324,7 @@ main(int argc, char *argv[])
 
     daemonize_complete();
 
-    idl = ovsdb_idl_create(remote, &ovsrec_idl_class);
+    idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true);
 
     for (;;) {
         const struct ovsrec_open_vswitch *ovs;