[ovs-dev] [PATCH 1/2] ovsdb-idl: Make it possible to omit or pay less attention to columns.
Ben Pfaff
blp at nicira.com
Mon Jul 19 23:30:29 UTC 2010
ovs-vswitchd has no need to replicate some parts of the database. In
particular, it doesn't need to replicate the bits that it never reads,
such as the external_ids column in the Open_vSwitch table. This saves
some memory, CPU time, and bandwidth to the database.
Another type of column that benefits from special treatment is "write-only
columns", that is, those that ovs-vswitchd writes and keeps up-to-date but
never expects another client to write, such as the cur_cfg column in the
Open_vSwitch table. If the IDL reports that the database has changed when
ovs-vswitchd updates such a column, then ovs-vswitchd reconfigures itself
for no reason, wasting CPU time. This commit also adds support for such
columns.
---
lib/ovsdb-idl-provider.h | 21 +++++++++
lib/ovsdb-idl.c | 111 +++++++++++++++++++++++++++++++++++++++-------
lib/ovsdb-idl.h | 4 ++
3 files changed, 119 insertions(+), 17 deletions(-)
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index c86396c..3f75b66 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -52,8 +52,29 @@ 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. */
+ 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. */
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 f32fe0e..9cd3d59 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -112,13 +112,13 @@ static void ovsdb_idl_send_monitor_request(struct ovsdb_idl *);
static void ovsdb_idl_parse_update(struct ovsdb_idl *, const struct json *);
static struct ovsdb_error *ovsdb_idl_parse_update__(struct ovsdb_idl *,
const struct json *);
-static void ovsdb_idl_process_update(struct ovsdb_idl_table *,
+static bool ovsdb_idl_process_update(struct ovsdb_idl_table *,
const struct uuid *,
const struct json *old,
const struct json *new);
static void ovsdb_idl_insert_row(struct ovsdb_idl_row *, const struct json *);
static void ovsdb_idl_delete_row(struct ovsdb_idl_row *);
-static void ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct json *);
+static bool ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct json *);
static bool ovsdb_idl_row_is_orphan(const struct ovsdb_idl_row *);
static struct ovsdb_idl_row *ovsdb_idl_row_create__(
@@ -159,6 +159,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);
shash_init(&table->columns);
for (j = 0; j < tc->n_columns; j++) {
const struct ovsdb_idl_column *column = &tc->columns[j];
@@ -189,6 +191,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl)
struct ovsdb_idl_table *table = &idl->tables[i];
shash_destroy(&table->columns);
hmap_destroy(&table->rows);
+ free(table->modes);
}
shash_destroy(&idl->table_by_name);
free(idl->tables);
@@ -357,6 +360,68 @@ 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)
+{
+ size_t i;
+
+ 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;
+ }
+ }
+
+ 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'.
+ *
+ * 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.
+ *
+ * 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 should be called after ovsdb_idl_create(), but before the
+ * first call to ovsdb_idl_run(). This function is mutually exclusive with
+ * ovsdb_idl_omit() for any given column. */
+void
+ovsdb_idl_set_write_only(struct ovsdb_idl *idl,
+ const struct ovsdb_idl_column *column)
+{
+ ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_WO);
+}
+
+/* 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.
+ *
+ * This function should be called after ovsdb_idl_create(), but before the
+ * first call to ovsdb_idl_run().
+ *
+ * This function should be called after ovsdb_idl_create(), but before the
+ * first call to ovsdb_idl_run(). This function is mutually exclusive with
+ * ovsdb_idl_set_write_only() for any given column. */
+void
+ovsdb_idl_omit(struct ovsdb_idl *idl, const struct ovsdb_idl_column *column)
+{
+ ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_NONE);
+}
static void
ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
@@ -376,7 +441,9 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
columns = json_array_create_empty();
for (i = 0; i < tc->n_columns; i++) {
const struct ovsdb_idl_column *column = &tc->columns[i];
- json_array_add(columns, json_string_create(column->name));
+ if (table->modes[i] != OVSDB_IDL_MODE_NONE) {
+ 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);
@@ -394,11 +461,7 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
static void
ovsdb_idl_parse_update(struct ovsdb_idl *idl, const struct json *table_updates)
{
- struct ovsdb_error *error;
-
- idl->change_seqno++;
-
- error = ovsdb_idl_parse_update__(idl, table_updates);
+ struct ovsdb_error *error = ovsdb_idl_parse_update__(idl, table_updates);
if (error) {
if (!VLOG_DROP_WARN(&syntax_rl)) {
char *s = ovsdb_error_to_string(error);
@@ -478,7 +541,9 @@ ovsdb_idl_parse_update__(struct ovsdb_idl *idl,
"and \"new\" members");
}
- ovsdb_idl_process_update(table, &uuid, old_json, new_json);
+ if (ovsdb_idl_process_update(table, &uuid, old_json, new_json)) {
+ idl->change_seqno++;
+ }
}
}
@@ -499,7 +564,7 @@ ovsdb_idl_get_row(struct ovsdb_idl_table *table, const struct uuid *uuid)
return NULL;
}
-static void
+static bool
ovsdb_idl_process_update(struct ovsdb_idl_table *table,
const struct uuid *uuid, const struct json *old,
const struct json *new)
@@ -516,6 +581,7 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
VLOG_WARN_RL(&semantic_rl, "cannot delete missing row "UUID_FMT" "
"from table %s",
UUID_ARGS(uuid), table->class->name);
+ return false;
}
} else if (!old) {
/* Insert row. */
@@ -526,14 +592,14 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
} else {
VLOG_WARN_RL(&semantic_rl, "cannot add existing row "UUID_FMT" to "
"table %s", UUID_ARGS(uuid), table->class->name);
- ovsdb_idl_modify_row(row, new);
+ return ovsdb_idl_modify_row(row, new);
}
} else {
/* Modify row. */
if (row) {
/* XXX perhaps we should check the 'old' values? */
if (!ovsdb_idl_row_is_orphan(row)) {
- ovsdb_idl_modify_row(row, new);
+ return ovsdb_idl_modify_row(row, new);
} else {
VLOG_WARN_RL(&semantic_rl, "cannot modify missing but "
"referenced row "UUID_FMT" in table %s",
@@ -546,13 +612,16 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
ovsdb_idl_insert_row(ovsdb_idl_row_create(table, uuid), new);
}
}
+
+ return true;
}
-static void
+static bool
ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
{
struct ovsdb_idl_table *table = row->table;
struct shash_node *node;
+ bool changed = false;
SHASH_FOR_EACH (node, json_object(row_json)) {
const char *column_name = node->name;
@@ -569,9 +638,12 @@ ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
error = ovsdb_datum_from_json(&datum, &column->type, node->data, NULL);
if (!error) {
- ovsdb_datum_swap(&row->old[column - table->class->columns],
- &datum);
+ unsigned int column_idx = column - table->class->columns;
+ ovsdb_datum_swap(&row->old[column_idx], &datum);
ovsdb_datum_destroy(&datum, &column->type);
+ if (table->modes[column_idx] == OVSDB_IDL_MODE_RW) {
+ changed = true;
+ }
} else {
char *s = ovsdb_error_to_string(error);
VLOG_WARN_RL(&syntax_rl, "error parsing column %s in row "UUID_FMT
@@ -581,6 +653,7 @@ ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
ovsdb_error_destroy(error);
}
}
+ return changed;
}
/* When a row A refers to row B through a column with a "refTable" constraint,
@@ -787,13 +860,17 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
}
}
-static void
+static bool
ovsdb_idl_modify_row(struct ovsdb_idl_row *row, const struct json *row_json)
{
+ bool changed;
+
ovsdb_idl_row_unparse(row);
ovsdb_idl_row_clear_arcs(row, true);
- ovsdb_idl_row_update(row, row_json);
+ changed = ovsdb_idl_row_update(row, row_json);
ovsdb_idl_row_parse(row);
+
+ return changed;
}
static bool
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index c09ba9b..9179e38 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -53,6 +53,10 @@ 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_set_write_only(struct ovsdb_idl *,
+ const struct ovsdb_idl_column *);
+void ovsdb_idl_omit(struct ovsdb_idl *, const struct ovsdb_idl_column *);
+
const struct ovsdb_idl_row *ovsdb_idl_get_row_for_uuid(
const struct ovsdb_idl *, const struct ovsdb_idl_table_class *,
const struct uuid *);
--
1.7.1
More information about the dev
mailing list