[ovs-dev,1/1] ovsdb-idl: Add additional support for change tracking.

(Reposting this patch for RFC)

Ovsdb-idl notifies a client that something changed; it does not track
which table, row changed in what way (insert, modify or delete).
As a result, a client has to scan or reconfigure the entire idl after
ovsdb_idl_run(). This is presumably fine for typical ovs schemas where
tables are relatively small. In use-cases where ovsdb is used with
schemas that can have very large tables, the current ovsdb-idl
notification mechanism does not appear to scale - clients need to do a
lot of processing to determine the exact change delta.

This change adds support for:
  - Table and row based change sequence numbers to record the
     most recent IDL change sequence numbers associated with insert,
     modify or delete update on that table or row.
  - Change tracking of specific columns. This ensures that changed
     rows (inserted, modified, deleted) that have tracked columns, are
     tracked by IDL. The client can directly access the changed rows
     with get_first, get_next operations without the need to scan the
     entire table.
     The tracking functionality is not enabled by default and needs to
     be turned on per-column by the client after ovsdb_idl_create()
     and before ovsdb_idl_run().

    /* Example Usage */
    {
       idl = ovsdb_idl_create(...);
       /* Track specific columns */
       ovsdb_idl_track_add_column(idl, column);
       /* Or, track all columns */
       ovsdb_idl_track_add_all(idl);
       for (;;) {
         ovsdb_idl_run(idl);
         seqno = ovsdb_idl_get_seqno(idl);
         /* Process only the changed rows in Table FOO */
         FOO_FOR_EACH_TRACKED(row, idl) {
           /* Determine the type of change from the row seqnos */
           if (foo_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > seqno)) {
             /* row is deleted */
           } else if {foo_row_get_seqno(row, OVSDB_IDL_CHANGE_MODIFY) > seqno))
             /* row is modified */
           } else if {foo_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT) > seqno))
             /* row is inserted */
           }
         }
         /* All changes processed - clear the change track */
         ovsdb_idl_track_clear(idl);
       }
    }

Signed-off-by: Shad Ansari <shad.ansari@hp.com>
---
 lib/ovsdb-idl-provider.h |   5 ++
 lib/ovsdb-idl.c          | 197 +++++++++++++++++++++++++++++++++++++++++++----
 lib/ovsdb-idl.h          |  31 ++++++++
 ovsdb/ovsdb-idlc.in      |  32 ++++++++
 tests/ovsdb-idl.at       | 120 +++++++++++++++++++++++++++++
 tests/test-ovsdb.c       | 181 ++++++++++++++++++++++++++++++-------------
 6 files changed, 498 insertions(+), 68 deletions(-)


@@ -1600,6 +1613,70 @@ compare_link1(const void *a_, const void *b_)
 }
 
 static void
+print_idl_row_simple(const struct idltest_simple *s, int step)
+{
+    size_t i;
+
+    printf("%03d: i=%"PRId64" r=%g b=%s s=%s u="UUID_FMT" ia=[",
+           step, s->i, s->r, s->b ? "true" : "false",
+           s->s, UUID_ARGS(&s->u));
+    for (i = 0; i < s->n_ia; i++) {
+        printf("%s%"PRId64, i ? " " : "", s->ia[i]);
+    }
+    printf("] ra=[");
+    for (i = 0; i < s->n_ra; i++) {
+        printf("%s%g", i ? " " : "", s->ra[i]);
+    }
+    printf("] ba=[");
+    for (i = 0; i < s->n_ba; i++) {
+        printf("%s%s", i ? " " : "", s->ba[i] ? "true" : "false");
+    }
+    printf("] sa=[");
+    for (i = 0; i < s->n_sa; i++) {
+        printf("%s%s", i ? " " : "", s->sa[i]);
+    }
+    printf("] ua=[");
+    for (i = 0; i < s->n_ua; i++) {
+        printf("%s"UUID_FMT, i ? " " : "", UUID_ARGS(&s->ua[i]));
+    }
+    printf("] uuid="UUID_FMT"\n", UUID_ARGS(&s->header_.uuid));
+}
+
+static void
+print_idl_row_link1(const struct idltest_link1 *l1, int step)
+{
+    struct idltest_link1 **links;
+    size_t i;
+
+    printf("%03d: i=%"PRId64" k=", step, l1->i);
+    if (l1->k) {
+        printf("%"PRId64, l1->k->i);
+    }
+    printf(" ka=[");
+    links = xmemdup(l1->ka, l1->n_ka * sizeof *l1->ka);
+    qsort(links, l1->n_ka, sizeof *links, compare_link1);
+    for (i = 0; i < l1->n_ka; i++) {
+        printf("%s%"PRId64, i ? " " : "", links[i]->i);
+    }
+    free(links);
+    printf("] l2=");
+    if (l1->l2) {
+        printf("%"PRId64, l1->l2->i);
+    }
+    printf(" uuid="UUID_FMT"\n", UUID_ARGS(&l1->header_.uuid));
+}
+
+static void
+print_idl_row_link2(const struct idltest_link2 *l2, int step)
+{
+    printf("%03d: i=%"PRId64" l1=", step, l2->i);
+    if (l2->l1) {
+        printf("%"PRId64, l2->l1->i);
+    }
+    printf(" uuid="UUID_FMT"\n", UUID_ARGS(&l2->header_.uuid));
+}
+
+static void
 print_idl(struct ovsdb_idl *idl, int step)
 {
     const struct idltest_simple *s;
@@ -1608,61 +1685,40 @@ print_idl(struct ovsdb_idl *idl, int step)
     int n = 0;
 
     IDLTEST_SIMPLE_FOR_EACH (s, idl) {
-        size_t i;
-
-        printf("%03d: i=%"PRId64" r=%g b=%s s=%s u="UUID_FMT" ia=[",
-               step, s->i, s->r, s->b ? "true" : "false",
-               s->s, UUID_ARGS(&s->u));
-        for (i = 0; i < s->n_ia; i++) {
-            printf("%s%"PRId64, i ? " " : "", s->ia[i]);
-        }
-        printf("] ra=[");
-        for (i = 0; i < s->n_ra; i++) {
-            printf("%s%g", i ? " " : "", s->ra[i]);
-        }
-        printf("] ba=[");
-        for (i = 0; i < s->n_ba; i++) {
-            printf("%s%s", i ? " " : "", s->ba[i] ? "true" : "false");
-        }
-        printf("] sa=[");
-        for (i = 0; i < s->n_sa; i++) {
-            printf("%s%s", i ? " " : "", s->sa[i]);
-        }
-        printf("] ua=[");
-        for (i = 0; i < s->n_ua; i++) {
-            printf("%s"UUID_FMT, i ? " " : "", UUID_ARGS(&s->ua[i]));
-        }
-        printf("] uuid="UUID_FMT"\n", UUID_ARGS(&s->header_.uuid));
+        print_idl_row_simple(s, step);
         n++;
     }
     IDLTEST_LINK1_FOR_EACH (l1, idl) {
-        struct idltest_link1 **links;
-        size_t i;
-
-        printf("%03d: i=%"PRId64" k=", step, l1->i);
-        if (l1->k) {
-            printf("%"PRId64, l1->k->i);
-        }
-        printf(" ka=[");
-        links = xmemdup(l1->ka, l1->n_ka * sizeof *l1->ka);
-        qsort(links, l1->n_ka, sizeof *links, compare_link1);
-        for (i = 0; i < l1->n_ka; i++) {
-            printf("%s%"PRId64, i ? " " : "", links[i]->i);
-        }
-        free(links);
-        printf("] l2=");
-        if (l1->l2) {
-            printf("%"PRId64, l1->l2->i);
-        }
-        printf(" uuid="UUID_FMT"\n", UUID_ARGS(&l1->header_.uuid));
+        print_idl_row_link1(l1, step);
         n++;
     }
     IDLTEST_LINK2_FOR_EACH (l2, idl) {
-        printf("%03d: i=%"PRId64" l1=", step, l2->i);
-        if (l2->l1) {
-            printf("%"PRId64, l2->l1->i);
-        }
-        printf(" uuid="UUID_FMT"\n", UUID_ARGS(&l2->header_.uuid));
+        print_idl_row_link2(l2, step);
+        n++;
+    }
+    if (!n) {
+        printf("%03d: empty\n", step);
+    }
+}
+
+static void
+print_idl_track(struct ovsdb_idl *idl, int step)
+{
+    const struct idltest_simple *s;
+    const struct idltest_link1 *l1;
+    const struct idltest_link2 *l2;
+    int n = 0;
+
+    IDLTEST_SIMPLE_FOR_EACH_TRACKED (s, idl) {
+        print_idl_row_simple(s, step);
+        n++;
+    }
+    IDLTEST_LINK1_FOR_EACH_TRACKED (l1, idl) {
+        print_idl_row_link1(l1, step);
+        n++;
+    }
+    IDLTEST_LINK2_FOR_EACH_TRACKED (l2, idl) {
+        print_idl_row_link2(l2, step);
         n++;
     }
     if (!n) {
@@ -1882,9 +1938,12 @@ do_idl(struct ovs_cmdl_context *ctx)
     int step = 0;
     int error;
     int i;
+    bool track;
 
     idltest_init();
 
+    track = ((struct test_ovsdb_pvt_context *)(ctx->pvt))->track;
+
     idl = ovsdb_idl_create(ctx->argv[1], &idltest_idl_class, true, true);
     if (ctx->argc > 2) {
         struct stream *stream;
@@ -1899,6 +1958,10 @@ do_idl(struct ovs_cmdl_context *ctx)
         rpc = NULL;
     }
 
+    if (track) {
+        ovsdb_idl_track_add_all(idl);
+    }
+
     setvbuf(stdout, NULL, _IONBF, 0);
 
     symtab = ovsdb_symbol_table_create();
@@ -1924,7 +1987,12 @@ do_idl(struct ovs_cmdl_context *ctx)
             }
 
             /* Print update. */
-            print_idl(idl, step++);
+            if (track) {
+                print_idl_track(idl, step++);
+                ovsdb_idl_track_clear(idl);
+            } else {
+                print_idl(idl, step++);
+            }
         }
         seqno = ovsdb_idl_get_seqno(idl);
 
@@ -1964,6 +2032,9 @@ do_idl(struct ovs_cmdl_context *ctx)
         poll_block();
     }
     print_idl(idl, step++);
+    if (track) {
+        ovsdb_idl_track_clear(idl);
+    }
     ovsdb_idl_destroy(idl);
     printf("%03d: done\n", step);
 }


[ovs-dev,1/1] ovsdb-idl: Add additional support for change tracking.

Commit Message

Comments

Patch