Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototype the deregister namespace functionality. #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/cm_stats_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ stats_ns_t *
stats_ns_t *
stats_register_ns(stats_recorder_t *, stats_ns_t *, const char *);

/* Deregister and free a namespace.
* Results are undefined if ns is still being used during or after this call
* if the return value is true.
* This function deregisters ns from parent within rec. If successful, ns is
* freed.
*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about separating the deregister from the free? I'm thinking we could deregister a namespace as part of a tear-down process, then fold the statistics under that namespace into a namespace dedicated to historical data (accumulating all the stuff that had been torn down in the past), and then free. It also provides a mechanism to move metrics from one location to another, like if we wanted to have "in-progress" metrics retrievable, and then committed to a new place?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would mean we have to expose the free call making the internal mechanics a bit more accessible. Given that you cannot be writing to this namespace when you deregister it (unsafe)... I think you could just merge the namespace into a new one then deregister w/ free as it is.

I'm not sure there is much advantage to giving that control. If you feel you need it, implement it and glue it into this PR.

bool
stats_deregister_ns(stats_recorder_t *rec, stats_ns_t *parent, stats_ns_t *ns);

typedef void (*stats_ns_update_func_t)(stats_ns_t *, void *closure);

/* Functions registered in this way will fire before a namespace is walked for
Expand Down
101 changes: 85 additions & 16 deletions src/stats_impl.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ static inline int __get_fanout(int fanout) {

struct stats_recorder_t {
struct stats_ns_t *global;
pthread_rwlock_t free_lock;
};
struct stats_ns_freshnode {
stats_ns_update_func_t f;
Expand Down Expand Up @@ -104,7 +105,7 @@ struct stats_handle_t {
typedef struct stats_container_t {
stats_ns_t *ns;
stats_handle_t *handle;
const char *key;
char *key;
int len;
} stats_container_t;

Expand Down Expand Up @@ -132,12 +133,51 @@ static bool hs_compare(const void *previous, const void *compare) {
return false;
}

static void
stats_container_free(stats_container_t *c) {
free(c->key);
free(c);
}

static void
stats_handle_free(stats_handle_t *h) {
if(h == NULL) return;
if(h->type == STATS_TYPE_HISTOGRAM_FAST) {
int i;
for(i=0;i<h->fanout;i++) {
hist_free(h->fan[i].cpu.hist);
pthread_mutex_destroy(&h->fan[i].cpu.mutex);
}
hist_free(h->hist_aggr);
}
else if(h->type == STATS_TYPE_HISTOGRAM) {
int i;
for(i=0;i<h->fanout;i++) {
hist_free(h->fan[i].cpu.hist);
pthread_mutex_destroy(&h->fan[i].cpu.mutex);
}
hist_free(h->hist_aggr);
}
free(h->fan);
pthread_mutex_destroy(&h->mutex);
}

static void
stats_ns_free(stats_ns_t *ns) {
int cleared = 0;
void *vc;
ck_hs_iterator_t iterator = CK_HS_ITERATOR_INITIALIZER;
if(ns == NULL) return;
while(ck_hs_next(&ns->map, &iterator, &vc)) {
stats_container_t *c = vc;
if(c->ns) stats_ns_free(c->ns);
if(c->handle) stats_handle_free(c->handle);
stats_container_free(c);
}
pthread_mutex_destroy(&ns->lock);
free(ns);
}

static stats_ns_t *
stats_ns_alloc(stats_recorder_t *rec) {
stats_ns_t *ns = calloc(1, sizeof(*ns));
Expand Down Expand Up @@ -171,6 +211,7 @@ stats_recorder_t *
stats_recorder_alloc(void) {
stats_recorder_t *rec = calloc(1, sizeof(*rec));
rec->global = stats_ns_alloc(rec);
pthread_rwlock_init(&rec->free_lock, NULL);
return rec;
}

Expand All @@ -186,7 +227,7 @@ stats_ns_add_container(stats_ns_t *ns, const char *name) {

if(strchr(name, '"')) return NULL;

nc.key = name;
nc.key = (char *)name;
nc.len = strlen(name);
// hashv won't change
hashv = CK_HS_HASH(&ns->map, hs_hash, &nc);
Expand All @@ -204,13 +245,49 @@ stats_ns_add_container(stats_ns_t *ns, const char *name) {
}
pthread_mutex_unlock(&ns->lock);
if(toadd) {
free((void *)toadd->key);
free(toadd->key);
free(toadd);
}
}
} while(prev == NULL);
return prev;
}
bool
stats_deregister_ns(stats_recorder_t *rec, stats_ns_t *parent, stats_ns_t *ns) {
bool success = false;
if(rec == NULL && parent != NULL) rec = parent->rec;
if(rec == NULL && ns != NULL) rec = ns->rec;
if(parent == NULL && rec != NULL) parent = rec->global;
if(rec == NULL || parent == NULL || ns == NULL) return false;
if(parent->rec != rec || ns->rec != rec) return false;

pthread_rwlock_wrlock(&rec->free_lock);
pthread_mutex_lock(&parent->lock);
pthread_mutex_lock(&ns->lock);

long hashv = CK_HS_HASH(&parent->map, hs_hash, ns);
stats_container_t *container = ck_hs_get(&parent->map, hashv, ns);
/* We look it up by name, but it must be the actual namespace we passed in */
if(container && container->ns == ns) {
ck_hs_remove(&parent->map, hashv, ns);
} else {
/* This can happen if someon has a.b.foo and a.c.foo and they attempt
* to free a.c.foo from a.b parent... "foo" matches, but it isn't the
* right namespace. */
container = NULL;
}

pthread_mutex_unlock(&ns->lock);
pthread_mutex_unlock(&parent->lock);
pthread_rwlock_unlock(&rec->free_lock);

if(container) { /* it was actually removed */
stats_ns_free(container->ns);
stats_container_free(container);
success = true;
}
return success;
}

stats_ns_t *
stats_register_ns(stats_recorder_t *rec, stats_ns_t *ns, const char *name) {
Expand Down Expand Up @@ -297,18 +374,6 @@ stats_handle_alloc(stats_ns_t *ns, stats_type_t type, int fanout) {
return h;
}

static void
stats_handle_free(stats_handle_t *h) {
int i;
if(h == NULL) return;
for(i=0;i<h->fanout;i++) {
if(h->fan[i].cpu.hist) hist_free(h->fan[i].cpu.hist);
}
free(h->fan);
if(h->hist_aggr) hist_free(h->hist_aggr);
free(h);
}

stats_handle_t *
stats_register_fanout(stats_ns_t *ns, const char *name, stats_type_t type, int fanout) {
stats_container_t *c;
Expand Down Expand Up @@ -770,5 +835,9 @@ ssize_t
stats_recorder_output_json(stats_recorder_t *rec,
bool hist_since_last, bool simple,
ssize_t (*outf)(void *, const char *, size_t), void *cl) {
return stats_con_output_json(rec->global, NULL, hist_since_last, simple, outf, cl);
ssize_t rv;
pthread_rwlock_rdlock(&rec->free_lock);
rv = stats_con_output_json(rec->global, NULL, hist_since_last, simple, outf, cl);
pthread_rwlock_unlock(&rec->free_lock);
return rv;
}