Change in common pattern groups get missed by router_printdiffs

[iain-buclaw-sociomantic]

When changing an aggregate from e.g:

aggregate
        ^(foo|bar)\.some_instance\.([^.]+)\.(.+)
    every 30 seconds
    expire after 90 seconds
    compute sum write to
        sums.some_instance.\2.\3
    send to graphite
    ;

To the following:

aggregate
        ^(foo|bar|baz)\.some_instance\.([^.]+)\.(.+)
    every 30 seconds
    expire after 90 seconds
    compute sum write to
        sums.some_instance.\2.\3
    send to graphite
    ;

The relay doesn't detect the change after signalling HUP to the program, because as far as it's concerned, the generated diff hasn't changed from:

# common pattern group 'some_instance' contains 8 aggregations/matches

This is pretty edge-case, as I have common metrics, but want to group only specific servers together.

[iain-buclaw-sociomantic]

A very quick patch that makes relay detect the change. Using an enum makes things clearer, though I'm no good at inventing names for these things. :-)

diff --git a/relay.c b/relay.c
index 5eb2243..2b12f02 100644
--- a/relay.c
+++ b/relay.c
@@ -684,11 +684,11 @@ main(int argc, char * const argv[])
                "(%zu aggregations with %zu computations omitted "
                "for brevity)\n",
                numaggregators, aggregator_numcomputes(aggrs));
-       router_printconfig(rtr, relay_stdout, 0);
+       router_printconfig(rtr, relay_stdout, RP_BRIEF);
    } else {
        fprintf(relay_stdout, "parsed configuration follows:\n");
        router_printconfig(rtr, relay_stdout,
-               1 + (mode & MODE_DEBUG ? 2 : 0));
+               (mode & MODE_DEBUG ? RP_DEBUG : RP_NORMAL));
    }
    fprintf(relay_stdout, "\n");

diff --git a/router.c b/router.c
index 7c5bd82..6a63570 100644
--- a/router.c
+++ b/router.c
@@ -2244,7 +2244,7 @@ router_getcollectorstub(router *rtr)
  * thousands.
  */
 void
-router_printconfig(router *rtr, FILE *f, char pmode)
+router_printconfig(router *rtr, FILE *f, router_pmode pmode)
 {
    cluster *c;
    route *r;
@@ -2290,7 +2290,7 @@ router_printconfig(router *rtr, FILE *f, char pmode)
                        PPROTO);
        }
        fprintf(f, "    ;\n");
-       if (pmode & 2) {
+       if (pmode == RP_DEBUG) {
            if (c->type == CARBON_CH ||
                    c->type == FNV1A_CH ||
                    c->type == JUMP_CH)
@@ -2308,10 +2308,10 @@ router_printconfig(router *rtr, FILE *f, char pmode)
            char stubname[48];
            char percentile[16];

-           if (!(pmode & 1))
+           if (pmode == RP_BRIEF)
                continue;

-           if (pmode & 2 || r->dests->next == NULL) {
+           if (pmode == RP_DEBUG || r->dests->next == NULL) {
                stubname[0] = '\0';
            } else {
                snprintf(stubname, sizeof(stubname),
@@ -2352,7 +2352,7 @@ router_printconfig(router *rtr, FILE *f, char pmode)
                        "<unknown>",
                        ac->metric + strlen(stubname));
            }
-           if (!(pmode & 2) && r->dests->next != NULL) {
+           if (!(pmode == RP_DEBUG) && r->dests->next != NULL) {
                destinations *dn = r->dests->next;
                fprintf(f, "    send to");
                if (dn->next == NULL) {
@@ -2383,10 +2383,15 @@ router_printconfig(router *rtr, FILE *f, char pmode)
            fprintf(f, "# common pattern group '%s' "
                    "contains %zu aggregations/matches\n",
                    ++b, cnt);
+
+           if (pmode == RP_DIFF) {
+               for (rwalk = r->dests->cl->members.routes; rwalk != NULL; rwalk = rwalk->next)
+                   fprintf(f, "        %s\n", rwalk->pattern);
+           }
        } else if (r->dests->cl->type == AGGRSTUB ||
                r->dests->cl->type == STATSTUB)
        {
-           if (pmode & 2) {
+           if (pmode == RP_DEBUG) {
                fprintf(f, "# stub match for aggregate/statistics rule "
                        "with send to\n");
                fprintf(f, "match ^%s\n    send to", r->pattern);
@@ -2474,7 +2479,7 @@ router_printdiffs(router *old, router *new, FILE *out)
                patho, strerror(errno));
        return 1;
    }
-   router_printconfig(old, f, 1);
+   router_printconfig(old, f, RP_DIFF);
    fclose(f);

    snprintf(pathn, sizeof(pathn), "%s/carbon-c-relay_route.XXXXXX", tmp);
@@ -2489,7 +2494,7 @@ router_printdiffs(router *old, router *new, FILE *out)
                pathn, strerror(errno));
        return 1;
    }
-   router_printconfig(new, f, 1);
+   router_printconfig(new, f, RP_DIFF);
    fclose(f);

    /* diff and print its output */
diff --git a/router.h b/router.h
index a4a19a2..ecef234 100644
--- a/router.h
+++ b/router.h
@@ -32,6 +32,8 @@ typedef struct {

 typedef struct _router router;

+typedef enum { RP_BRIEF, RP_NORMAL, RP_DEBUG, RP_DIFF } router_pmode;
+
 #define RE_MAX_MATCHES     64

 router *router_readconfig(router *orig, const char *path, size_t queuesize, size_t batchsize, int maxstalls, unsigned short iotimeout, unsigned int sockbufsize);
@@ -40,7 +42,7 @@ char router_printdiffs(router *old, router *new, FILE *out);
 void router_transplant_queues(router *new, router *old);
 char router_start(router *r);
 size_t router_rewrite_metric(char (*newmetric)[METRIC_BUFSIZ], char **newfirstspace, const char *metric, const char *firstspace, const char *replacement, const size_t nmatch, const regmatch_t *pmatch);
-void router_printconfig(router *r, FILE *f, char mode);
+void router_printconfig(router *r, FILE *f, router_pmode pmode);
 char router_route(router *r, destination ret[], size_t *retcnt, size_t retsize, char *srcaddr, char *metric, char *firstspace);
 void router_test(router *r, char *metric_path);
 server **router_getservers(router *r);

[grobian]

I used the numbers as bitflags, so you'd have to use defines instead. I've pushed the fix for your problem.