Skip to content

Commit

Permalink
Merge pull request #1511 from eladpe/v1.2
Browse files Browse the repository at this point in the history 
UCS/STATS: missing lock (v1.2)
  • Loading branch information
@yosefe
yosefe authored May 14, 2017
2 parents f4f44ef + b62de0e commit 257d38f
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 59 deletions.
59 changes: 35 additions & 24 deletions src/ucs/stats/stats.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,6 @@ ucs_sys_futex(volatile void *addr1, int op, int val1, struct timespec *timeout,
return syscall(SYS_futex, addr1, op, val1, timeout, uaddr2, val3);
}

static int ucs_stats_node_add(ucs_stats_node_t *node, ucs_stats_node_t *parent)
{
ucs_assert(node != &ucs_stats_context.root_node);
if (parent == NULL) {
return UCS_ERR_INVALID_PARAM;
}

/* Append node to existing tree */
pthread_mutex_lock(&ucs_stats_context.lock);
ucs_list_add_tail(&parent->children[UCS_STATS_ACTIVE_CHILDREN], &node->list);
node->parent = parent;

pthread_mutex_unlock(&ucs_stats_context.lock);
return UCS_OK;
}

static void ucs_stats_clean_node(ucs_stats_node_t *node) {
ucs_stats_filter_node_t * temp_filter_node;
ucs_stats_filter_node_t * filter_node;
Expand Down Expand Up @@ -241,15 +225,14 @@ static ucs_stats_filter_node_t * ucs_stats_find_class(ucs_stats_filter_node_t *f
}

static void ucs_stats_add_to_filter(ucs_stats_node_t *node,
ucs_stats_filter_node_t* filter_tree)
ucs_stats_filter_node_t * new_filter_node)
{
ucs_stats_filter_node_t *temp_filter_node;
ucs_stats_filter_node_t *filter_node = NULL;
ucs_stats_filter_node_t *filter_parent;
int found = 0;
int filter_index = 0;
int i;
ucs_status_t status;

if (ucs_global_opts.stats_format == UCS_STATS_SUMMARY) {
filter_parent = &ucs_stats_context.root_filter_node;
Expand All @@ -262,10 +245,7 @@ static void ucs_stats_add_to_filter(ucs_stats_node_t *node,
}

if (!filter_node) {
status = ucs_stats_filter_node_new(node->cls, &filter_node);
if (status != UCS_OK) {
return;
}
filter_node = new_filter_node;

filter_node->type_list_len = 0;
filter_node->ref_count = 0;
Expand Down Expand Up @@ -298,10 +278,31 @@ static void ucs_stats_add_to_filter(ucs_stats_node_t *node,
}
}

static int ucs_stats_node_add(ucs_stats_node_t *node,
ucs_stats_node_t *parent,
ucs_stats_filter_node_t *filter_node)
{
ucs_assert(node != &ucs_stats_context.root_node);
if (parent == NULL) {
return UCS_ERR_INVALID_PARAM;
}

/* Append node to existing tree */
pthread_mutex_lock(&ucs_stats_context.lock);
ucs_list_add_tail(&parent->children[UCS_STATS_ACTIVE_CHILDREN], &node->list);
node->parent = parent;
ucs_stats_add_to_filter(node, filter_node);

pthread_mutex_unlock(&ucs_stats_context.lock);

return UCS_OK;
}

ucs_status_t ucs_stats_node_alloc(ucs_stats_node_t** p_node, ucs_stats_class_t *cls,
ucs_stats_node_t *parent, const char *name, ...)
{
ucs_stats_node_t *node;
ucs_stats_filter_node_t *filter_node;
ucs_status_t status;
va_list ap;

Expand All @@ -324,15 +325,25 @@ ucs_status_t ucs_stats_node_alloc(ucs_stats_node_t** p_node, ucs_stats_class_t *
return status;
}

status = ucs_stats_filter_node_new(node->cls, &filter_node);
if (status != UCS_OK) {
ucs_free(node);
return status;
}

ucs_trace("allocated stats node '"UCS_STATS_NODE_FMT"'", UCS_STATS_NODE_ARG(node));

status = ucs_stats_node_add(node, parent);
status = ucs_stats_node_add(node, parent, filter_node);
if (status != UCS_OK) {
ucs_free(node);
ucs_free(filter_node);
return status;
}

ucs_stats_add_to_filter(node, &ucs_stats_context.root_filter_node);
if (node->filter_node != filter_node) {
ucs_free(filter_node);
}

*p_node = node;
return UCS_OK;
}
Expand Down
99 changes: 64 additions & 35 deletions test/gtest/ucs/test_stats.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ extern "C" {
#include <netinet/in.h>

#if ENABLE_STATS
#define NUM_DATA_NODES 20

class stats_test : public ucs::test {
public:
Expand Down Expand Up @@ -43,7 +44,8 @@ class stats_test : public ucs::test {
virtual std::string stats_dest_config() = 0;
virtual std::string stats_trigger_config() = 0;

void prepare_nodes() {
void prepare_nodes(ucs_stats_node_t **cat_node,
ucs_stats_node_t *data_nodes[NUM_DATA_NODES]) {
static stats_class<0> category_stats_class = {
{"category", 0, {}}
};
Expand All @@ -53,13 +55,13 @@ class stats_test : public ucs::test {
{ "counter0","counter1","counter2","counter3" }
};

ucs_status_t status = UCS_STATS_NODE_ALLOC(&cat_node,
ucs_status_t status = UCS_STATS_NODE_ALLOC(cat_node,
&category_stats_class.cls,
ucs_stats_get_root());
ASSERT_UCS_OK(status);
for (unsigned i = 0; i < NUM_DATA_NODES; ++i) {
status = UCS_STATS_NODE_ALLOC(&data_nodes[i], &data_stats_class.cls,
cat_node, "-%d", i);
*cat_node, "-%d", i);
ASSERT_UCS_OK(status);

UCS_STATS_UPDATE_COUNTER(data_nodes[i], 0, 10);
Expand All @@ -69,23 +71,26 @@ class stats_test : public ucs::test {
}

/* make sure our original node is ok */
check_cat_node(cat_node);
check_cat_node(*cat_node, data_nodes);
}

void free_nodes() {
void free_nodes(ucs_stats_node_t *cat_node,
ucs_stats_node_t *data_nodes[NUM_DATA_NODES]) {
for (unsigned i = 0; i < NUM_DATA_NODES; ++i) {
UCS_STATS_NODE_FREE(data_nodes[i]);
}
UCS_STATS_NODE_FREE(cat_node);
}

void check_tree(ucs_stats_node_t *root) {
void check_tree(ucs_stats_node_t *root,
ucs_stats_node_t *data_nodes[NUM_DATA_NODES]) {
EXPECT_EQ(1ul, ucs_list_length(&root->children[UCS_STATS_ACTIVE_CHILDREN]));
check_cat_node(ucs_list_head(&root->children[UCS_STATS_ACTIVE_CHILDREN],
ucs_stats_node_t, list));
ucs_stats_node_t, list), data_nodes);
}

void check_cat_node(ucs_stats_node_t *cat_node) {
void check_cat_node(ucs_stats_node_t *cat_node,
ucs_stats_node_t *data_nodes[NUM_DATA_NODES]) {
EXPECT_EQ(std::string("category"), std::string(cat_node->cls->name));
EXPECT_EQ((unsigned)0, cat_node->cls->num_counters);

Expand All @@ -103,11 +108,7 @@ class stats_test : public ucs::test {
}

protected:
static const unsigned NUM_DATA_NODES = 20;
static const unsigned NUM_COUNTERS = 4;

ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES];
};

class stats_udp_test : public stats_test {
Expand Down Expand Up @@ -139,11 +140,11 @@ class stats_udp_test : public stats_test {
return "timer:0.1s";
}

void read_and_check_stats() {
void read_and_check_stats(ucs_stats_node_t *data_nodes[NUM_DATA_NODES]) {
ucs_list_link_t *list = ucs_stats_server_get_stats(m_server);
ucs_assert(1ul == ucs_list_length(list));
ASSERT_EQ(1ul, ucs_list_length(list));
check_tree(ucs_list_head(list, ucs_stats_node_t, list));
check_tree(ucs_list_head(list, ucs_stats_node_t, list), data_nodes);
ucs_stats_server_purge_stats(m_server);
}

Expand Down Expand Up @@ -247,27 +248,35 @@ class stats_on_exit_test : public stats_file_test {
};

UCS_TEST_F(stats_on_demand_test, null_root) {
static stats_class<0> category_stats_class = {
{"category", 0, {}}
};
ucs_status_t status = UCS_STATS_NODE_ALLOC(&cat_node,
&category_stats_class.cls,
NULL);
ucs_stats_node_t *cat_node;

static stats_class<0> category_stats_class = {
{"category", 0, {}}
};
ucs_status_t status = UCS_STATS_NODE_ALLOC(&cat_node,
&category_stats_class.cls,
NULL);

EXPECT_GE(status, UCS_ERR_INVALID_PARAM);
EXPECT_GE(status, UCS_ERR_INVALID_PARAM);
}

UCS_TEST_F(stats_udp_test, report) {
prepare_nodes();
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};

prepare_nodes(&cat_node, data_nodes);
wait_for_stats();
read_and_check_stats();
free_nodes();
read_and_check_stats(data_nodes);
free_nodes(cat_node, data_nodes);
}

UCS_TEST_F(stats_file_test, report) {
prepare_nodes();
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};

prepare_nodes(&cat_node, data_nodes);
ucs_stats_dump();
free_nodes();
free_nodes(cat_node, data_nodes);

std::string data = get_data();
FILE *f = fmemopen(&data[0], data.size(), "rb");
Expand All @@ -276,29 +285,49 @@ UCS_TEST_F(stats_file_test, report) {
ASSERT_UCS_OK(status);
fclose(f);

check_tree(root);
check_tree(root, data_nodes);
ucs_stats_free(root);
}

UCS_TEST_F(stats_on_demand_test, report) {
prepare_nodes();
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};

prepare_nodes(&cat_node, data_nodes);
ucs_stats_dump();
wait_for_stats();
read_and_check_stats();
free_nodes();
read_and_check_stats(data_nodes);
free_nodes(cat_node, data_nodes);
}

UCS_TEST_F(stats_on_signal_test, report) {
prepare_nodes();
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};

prepare_nodes(&cat_node, data_nodes);
kill(getpid(), SIGUSR1);
wait_for_stats();
read_and_check_stats();
free_nodes();
read_and_check_stats(data_nodes);
free_nodes(cat_node, data_nodes);
}

UCS_TEST_F(stats_on_exit_test, dump) {
prepare_nodes();
free_nodes();
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};

prepare_nodes(&cat_node, data_nodes);
free_nodes(cat_node, data_nodes);
}

UCS_MT_TEST_F(stats_file_test, mt_add_remove, 10) {
ucs_stats_node_t *cat_node;
ucs_stats_node_t *data_nodes[NUM_DATA_NODES] = {NULL};
unsigned i;

for (i = 0; i < 100; i++) {
prepare_nodes(&cat_node, data_nodes);
free_nodes(cat_node, data_nodes);
}
}

#endif

0 comments on commit 257d38f

Please sign in to comment.