[ovs-dev] [PATCH monitor_cond V5 13/18] ovsdb: adjust ovsdb_monitor_changes to hold rows by either transaction id or datum

Liran Schour lirans at il.ibm.com
Fri Mar 4 08:09:08 UTC 2016


The next few patchs implements a data structure to track rows according to matched
condition's clauses. The goal is to reduce the computation of conditional monitoring
when all clauses have "==" function (which is the most common case).
This patch adjust ovsdb_monitor_changes to hold rows either since transaction point in
time or hold rows according to a common column value. In the later case we need to
maintain a list of all old values that we have and set the right old value before
using ovsdb_monitor_row.

Signed-off-by: Liran Schour <lirans at il.ibm.com>
---
 ovsdb/monitor.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 96 insertions(+), 15 deletions(-)

diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 63d1ff3..41e0d93 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -103,31 +103,59 @@ struct ovsdb_monitor_column {
     bool monitored;
 };
 
-/* A row that has changed in a monitored table. */
+/* An old value of a row at transaction point in time */
+struct ovsdb_monitor_data {
+    struct ovs_list node;
+    uint64_t transaction;
+    struct ovsdb_datum *data;
+};
+
+/* A row that has changed in a monitored table.
+ * Being holded either by ovsdb_monitor_table.changes or
+ * by ovsdb_tracked_clauses.changes due to match of a session condition's
+ * clause with datum 'arg' on column ovsdb_tracked_clauses.column */
 struct ovsdb_monitor_row {
     struct hmap_node hmap_node; /* In ovsdb_jsonrpc_monitor_table.changes. */
     struct uuid uuid;           /* UUID of row that changed. */
+    uint64_t transaction;       /* Transaction ID at time of insertion
+                                 * 0 means row is being holded by
+                                 * ovsdb_monitor_table.changes */
+    struct ovs_list old_list;   /* Contains ovsdb_monitor_data sorted by
+                                   transaction. Valid only in case of clause
+                                   tracking */
     struct ovsdb_datum *old;    /* Old data, NULL for an inserted row. */
     struct ovsdb_datum *new;    /* New data, NULL for a deleted row. */
 };
 
 /* Contains 'struct ovsdb_monitor_row's for rows that have been
- * updated but not yet flushed to all the jsonrpc connection.
+ * updated in 2 cases:
+ * 1. Oridnary row change that is being held by ovsdb_monitor_table.changes.
+ *    In this case any row in changes is updated but  not yet flushed to all
+ *    the jsonrpc connection.
+ *    'transaction' stores the first update's transaction id.
+ *
+ * 2. Updated row that match a conditon's clause with datum
+ *    ovsdb_monitor_row.arg and column ovsdb_tracked_clauses.column
  *
- * 'n_refs' represent the number of jsonrpc connections that have
- * not received updates. Generate the update for the last jsonprc
- * connection will also destroy the whole "struct ovsdb_monitor_changes"
- * object.
+ * If changes is being held by ovsdb_monitor_table.changes, 'n_refs' represent
+ * the number of jsonrpc connections that have not received updates.
+ * In this case generate the update for the last jsonprc connection will also
+ * destroy the whole "struct ovsdb_monitor_changes" object.
+ * Otherwise, in case changes is being helg by ovsdb_tracked_clauses.changes.
+ * 'n_refs' represent the number of existing condition's clause with
+ * 'ovsdb_tracked_clauses.column' and 'ovsdb_monitor_changes.arg'.
  *
- * 'transaction' stores the first update's transaction id.
  * */
 struct ovsdb_monitor_changes {
     struct ovsdb_monitor_table *mt;
     struct hmap rows;
     int n_refs;
     uint64_t transaction;
+    const struct ovsdb_type *type;
+    struct ovsdb_datum arg;
     struct hmap_node hmap_node;  /* Element in ovsdb_monitor_tables' changes
-                                    hmap.  */
+                                  * hmap or in ovsdb_tracked_clauses' changes
+                                  * hmap. */
 };
 
 /* A particular table being monitored. */
@@ -178,6 +206,14 @@ static void ovsdb_monitor_changes_destroy(
                                   struct ovsdb_monitor_changes *changes);
 static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
                                   uint64_t unflushed);
+static void
+ovsdb_monitor_changes_update(const struct ovsdb_row *old,
+                             const struct ovsdb_row *new,
+                             const struct ovsdb_monitor_table *mt,
+                             struct ovsdb_monitor_changes *changes,
+                             uint64_t transaction);
+static void
+ovsdb_monitor_row_set_old(struct ovsdb_monitor_row *row, uint64_t transaction);
 
 static uint32_t
 json_cache_hash(enum ovsdb_monitor_version version, uint64_t from_txn)
@@ -348,7 +384,16 @@ ovsdb_monitor_row_destroy(const struct ovsdb_monitor_table *mt,
                           struct ovsdb_monitor_row *row)
 {
     if (row) {
-        free_monitor_row_data(mt, row->old);
+        if (!row->transaction) {
+            free_monitor_row_data(mt, row->old);
+        } else {
+            struct ovsdb_monitor_data *old;
+
+            LIST_FOR_EACH_POP(old, node, &row->old_list) {
+                free_monitor_row_data(mt, old->data);
+                free(old);
+            }
+        }
         free_monitor_row_data(mt, row->new);
         free(row);
     }
@@ -549,6 +594,8 @@ ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
     changes->transaction = next_txn;
     changes->mt = mt;
     changes->n_refs = 1;
+    ovsdb_datum_init_empty(&changes->arg); /*Dummy values */
+    changes->type = &ovsdb_type_boolean;
     hmap_init(&changes->rows);
     hmap_insert(&mt->changes, &changes->hmap_node, hash_uint64(next_txn));
 
@@ -612,6 +659,7 @@ ovsdb_monitor_changes_destroy(struct ovsdb_monitor_changes *changes)
         ovsdb_monitor_row_destroy(changes->mt, row);
     }
     hmap_destroy(&changes->rows);
+    ovsdb_datum_destroy(&changes->arg, changes->type);
     free(changes);
 }
 
@@ -1306,11 +1354,30 @@ ovsdb_monitor_init_aux(struct ovsdb_monitor_aux *aux,
     aux->efficacy = OVSDB_CHANGES_NO_EFFECT;
 }
 
+/* Set old row to an old value from list with the minimum transaction ID bigger
+ * then 'transaction' */
+static void
+ovsdb_monitor_row_set_old(struct ovsdb_monitor_row *row, uint64_t transaction)
+{
+    struct ovsdb_monitor_data *data;
+    ovs_assert(row->transaction);
+
+    LIST_FOR_EACH(data, node, &row->old_list) {
+        if (data->transaction >= transaction) {
+            break;
+        }
+    }
+    ovs_assert(data);
+
+    row->old = data->data;
+}
+
 static void
 ovsdb_monitor_changes_update(const struct ovsdb_row *old,
                              const struct ovsdb_row *new,
                              const struct ovsdb_monitor_table *mt,
-                             struct ovsdb_monitor_changes *changes)
+                             struct ovsdb_monitor_changes *changes,
+                             uint64_t transaction)
 {
     const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old);
     struct ovsdb_monitor_row *change;
@@ -1320,8 +1387,12 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old,
         change = xzalloc(sizeof *change);
         hmap_insert(&changes->rows, &change->hmap_node, uuid_hash(uuid));
         change->uuid = *uuid;
-        change->old = clone_monitor_row_data(mt, old);
+        change->transaction = transaction;
         change->new = clone_monitor_row_data(mt, new);
+        list_init(&change->old_list);
+        if (!transaction) {
+            change->old = clone_monitor_row_data(mt, old);
+        }
     } else {
         if (new) {
             update_monitor_row_data(mt, new, change->new);
@@ -1329,13 +1400,22 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old,
             free_monitor_row_data(mt, change->new);
             change->new = NULL;
 
-            if (!change->old) {
+            if (!transaction && !change->old) {
                 /* This row was added then deleted.  Forget about it. */
                 hmap_remove(&changes->rows, &change->hmap_node);
-                free(change);
+                ovsdb_monitor_row_destroy(mt, change);
+                return;
             }
         }
     }
+    if (transaction) {
+        /* Clauses tracked row - maintain old_list */
+        struct ovsdb_monitor_data *data = xzalloc(sizeof *data);
+
+        data->data = clone_monitor_row_data(mt, old);;
+        change->transaction = data->transaction = transaction;
+        list_push_back(&change->old_list, &data->node);
+    }
 }
 
 static bool
@@ -1411,9 +1491,10 @@ ovsdb_monitor_change_cb(const struct ovsdb_row *old,
     }
     HMAP_FOR_EACH(changes, hmap_node, &mt->changes) {
         if (efficacy > OVSDB_CHANGES_NO_EFFECT) {
-            ovsdb_monitor_changes_update(old, new, mt, changes);
+            ovsdb_monitor_changes_update(old, new, mt, changes, 0);
         }
     }
+
     if (aux->efficacy < efficacy) {
         aux->efficacy = efficacy;
     }
@@ -1437,7 +1518,7 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
             if (!changes) {
                 changes = ovsdb_monitor_table_add_changes(mt, 0);
                 HMAP_FOR_EACH (row, hmap_node, &mt->table->rows) {
-                    ovsdb_monitor_changes_update(NULL, row, mt, changes);
+                    ovsdb_monitor_changes_update(NULL, row, mt, changes, 0);
                 }
             } else {
                 changes->n_refs++;
-- 
2.1.4





More information about the dev mailing list