[ovs-dev] [upgrades 3/4] ovsdb-idl: Make selecting tables and columns to replicate more flexible.
Ben Pfaff
blp at nicira.com
Mon Aug 30 21:48:37 UTC 2010
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 | 25 +-------
lib/ovsdb-idl.c | 161 +++++++++++++++++++++++++++++++++-------------
lib/ovsdb-idl.h | 43 +++++++++++-
tests/test-ovsdb.c | 2 +-
utilities/ovs-vsctl.c | 2 +-
vswitchd/bridge.c | 10 ++--
vswitchd/ovs-brcompatd.c | 2 +-
7 files changed, 167 insertions(+), 78 deletions(-)
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 040a699..87f62d6 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -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. */
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 2132f9f..3abf4f0 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -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];
@@ -361,64 +378,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)
+
+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;
}
static void
@@ -435,16 +501,22 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
struct json *monitor_request, *columns;
size_t i;
- monitor_request = json_object_create();
- columns = json_array_create_empty();
+ columns = table->need_table ? json_array_create_empty() : NULL;
for (i = 0; i < tc->n_columns; i++) {
const struct ovsdb_idl_column *column = &tc->columns[i];
- if (table->modes[i] != OVSDB_IDL_MODE_NONE) {
+ if (table->modes[i] & 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);
@@ -645,7 +717,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 {
@@ -1558,7 +1630,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,
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 9179e38..9e3816b 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -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 *);
+
+/* 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 both 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 *);
+
+/* 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);
+
+/* Transactions. */
enum ovsdb_idl_txn_status {
TXN_UNCHANGED, /* Transaction didn't include any changes. */
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 18784a5..8660b3c 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1813,7 +1813,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;
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 4d50194..2df7d66 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -143,7 +143,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);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 952049e..db1583a 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -266,10 +266,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);
@@ -277,8 +277,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. */
diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c
index 0b5ebb9..950e12c 100644
--- a/vswitchd/ovs-brcompatd.c
+++ b/vswitchd/ovs-brcompatd.c
@@ -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;
--
1.7.1
More information about the dev
mailing list