2 a.m.
This is an automated email from the git hooks/post-receive script.
firstyear pushed a commit to branch master
in repository 389-ds-base.
commit 3d73e3877f162514f0c495c4adab8e6932edd808
Author: William Brown <firstyear(a)redhat.com>
Date: Fri Mar 10 13:46:57 2017 +1000
Ticket 49154 - Nunc Stans stress should assert it has 95% success rate
Bug Description: We should assert that the nunc-stans stress test
is able to pass 95% of it's connections during an overload scenario.
Fix Description: Assert we pass 95% of connections. Additionally, we
were not actually running the tests properly, so fix that. Improve
the the work thread function to be slightly faster by better using our
atomic shutdown check.
https://pagure.io/389-ds-base/issue/49154
Author: wibrown
Review by: vashirov (Thanks!)
---
Makefile.am | 21 ++++--
src/nunc-stans/ns/ns_thrpool.c | 54 +++++++-------
src/nunc-stans/test/test_nuncstans_stress.h | 42 +++++++++++
...stans_stress.c => test_nuncstans_stress_core.c} | 83 ++++++++--------------
src/nunc-stans/test/test_nuncstans_stress_large.c | 33 +++++++++
src/nunc-stans/test/test_nuncstans_stress_small.c | 33 +++++++++
6 files changed, 179 insertions(+), 87 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index ccbb530..1cf61cd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -553,7 +553,8 @@ dist_noinst_HEADERS += \
test/test_slapd.h \
src/libsds/test/test_sds.h \
src/libsds/test/benchmark.h \
- src/libsds/test/benchmark_par.h
+ src/libsds/test/benchmark_par.h \
+ src/nunc-stans/test/test_nuncstans_stress.h
endif
dist_noinst_DATA = \
@@ -1985,12 +1986,13 @@ check_PROGRAMS = test_slapd \
benchmark_sds \
benchmark_par_sds \
test_nuncstans \
- test_nuncstans_stress
+ test_nuncstans_stress_small \
+ test_nuncstans_stress_large
# Mark all check programs for testing
TESTS = test_slapd \
test_libsds \
test_nuncstans \
- test_nuncstans_stress
+ test_nuncstans_stress_small
test_slapd_SOURCES = test/main.c \
test/libslapd/test.c \
@@ -2034,10 +2036,15 @@ test_nuncstans_CPPFLAGS = $(AM_CPPFLAGS) $(CMOCKA_INCLUDES)
$(NUNCSTANS_CPPFLAGS
test_nuncstans_LDADD = libnunc-stans.la libsds.la
test_nuncstans_LDFLAGS = $(ASAN_DEFINES) $(PROFILING_LINKS) $(CMOCKA_LINKS)
$(EVENT_LINK)
-test_nuncstans_stress_SOURCES = src/nunc-stans/test/test_nuncstans.c
-test_nuncstans_stress_CPPFLAGS = $(AM_CPPFLAGS) $(CMOCKA_INCLUDES) $(NUNCSTANS_CPPFLAGS)
-test_nuncstans_stress_LDADD = libnunc-stans.la libsds.la
-test_nuncstans_stress_LDFLAGS = $(ASAN_DEFINES) $(PROFILING_LINKS) $(CMOCKA_LINKS)
$(EVENT_LINK)
+test_nuncstans_stress_large_SOURCES = src/nunc-stans/test/test_nuncstans_stress_large.c
src/nunc-stans/test/test_nuncstans_stress_core.c
+test_nuncstans_stress_large_CPPFLAGS = $(AM_CPPFLAGS) $(CMOCKA_INCLUDES)
$(NUNCSTANS_CPPFLAGS)
+test_nuncstans_stress_large_LDADD = libnunc-stans.la libsds.la
+test_nuncstans_stress_large_LDFLAGS = $(ASAN_DEFINES) $(PROFILING_LINKS) $(CMOCKA_LINKS)
$(EVENT_LINK)
+
+test_nuncstans_stress_small_SOURCES = src/nunc-stans/test/test_nuncstans_stress_small.c
src/nunc-stans/test/test_nuncstans_stress_core.c
+test_nuncstans_stress_small_CPPFLAGS = $(AM_CPPFLAGS) $(CMOCKA_INCLUDES)
$(NUNCSTANS_CPPFLAGS)
+test_nuncstans_stress_small_LDADD = libnunc-stans.la libsds.la
+test_nuncstans_stress_small_LDFLAGS = $(ASAN_DEFINES) $(PROFILING_LINKS) $(CMOCKA_LINKS)
$(EVENT_LINK)
endif
diff --git a/src/nunc-stans/ns/ns_thrpool.c b/src/nunc-stans/ns/ns_thrpool.c
index 744749b..10791ee 100644
--- a/src/nunc-stans/ns/ns_thrpool.c
+++ b/src/nunc-stans/ns/ns_thrpool.c
@@ -165,7 +165,6 @@ os_free(void *ptr)
int32_t
ns_thrpool_is_shutdown(struct ns_thrpool_t *tp)
{
- /* We need to barrier this somehow? */
int32_t result = 0;
__atomic_load(&(tp->shutdown), &result, __ATOMIC_SEQ_CST);
return result;
@@ -174,7 +173,6 @@ ns_thrpool_is_shutdown(struct ns_thrpool_t *tp)
int32_t
ns_thrpool_is_event_shutdown(struct ns_thrpool_t *tp)
{
- /* We need to barrier this somehow? */
int32_t result = 0;
__atomic_load(&(tp->shutdown_event_loop), &result, __ATOMIC_SEQ_CST);
return result;
@@ -239,6 +237,8 @@ internal_ns_job_rearm(ns_job_t *job)
#endif
job->state = NS_JOB_ARMED;
+ /* I think we need to check about is_shutdown here? */
+
if (NS_JOB_IS_IO(job->job_type) || NS_JOB_IS_TIMER(job->job_type) ||
NS_JOB_IS_SIGNAL(job->job_type)) {
event_q_notify(job);
} else {
@@ -274,21 +274,7 @@ work_job_execute(ns_job_t *job)
#ifdef DEBUG
ns_log(LOG_DEBUG, "work_job_execute PERSIST and RUNNING, remarking %x as
NS_JOB_NEEDS_ARM\n", job);
#endif
- /*
- * So at this point, if this is an IO or a SIGNAL job then, we are
- * still in the event framework's io event queue. So we actually
- * are already rearmed!!!
- *
- * This is *exactly* why it's impossible to disarm a persist IO job
- * once we start it from external threads! Too many dangers abound!
- */
- if (NS_JOB_IS_IO(job->job_type) || NS_JOB_IS_SIGNAL(job->job_type)) {
- job->state = NS_JOB_ARMED;
- pthread_mutex_unlock(job->monitor);
- return;
- } else {
- job->state = NS_JOB_NEEDS_ARM;
- }
+ job->state = NS_JOB_NEEDS_ARM;
}
if (job->state == NS_JOB_NEEDS_DELETE) {
@@ -360,6 +346,8 @@ worker_thread_func(void *arg)
{
ns_thread_t *thr = (ns_thread_t *)arg;
ns_thrpool_t *tp = thr->tp;
+ sds_result result = SDS_SUCCESS;
+ int32_t is_shutdown = ns_thrpool_is_shutdown(tp);
/* Get ready to use lock free ds */
sds_lqueue_tprep(tp->work_q);
@@ -367,18 +355,23 @@ worker_thread_func(void *arg)
/*
* Execute jobs until shutdown is set and the queues are empty.
*/
- while (!ns_thrpool_is_shutdown(tp)) {
+ while (!is_shutdown) {
ns_job_t *job = NULL;
+ result = sds_lqueue_dequeue(tp->work_q, (void **)&job);
/* Don't need monitor here, job_dequeue barriers the memory for us. Job will
be valid */
- while(sds_lqueue_dequeue(tp->work_q, (void **)&job) == SDS_LIST_EXHAUSTED
&& !ns_thrpool_is_shutdown(tp))
- {
+ /* Is it possible for a worker thread to get stuck here during shutdown? */
+ if (result == SDS_LIST_EXHAUSTED && !is_shutdown) {
work_q_wait(tp);
- }
-
- if (job) {
+ } else if (result == SDS_SUCCESS && job != NULL) {
+ /* Even if we are shutdown here, we can process a job. */
+ /* Should we just keep dequeing until we exhaust the list? */
work_job_execute(job);
/* MUST NOT ACCESS JOB FROM THIS POINT */
+ } else {
+ ns_log(LOG_ERR, "worker_thread_func encountered a recoverable issue
during processing of the queue\n");
}
+
+ is_shutdown = ns_thrpool_is_shutdown(tp);
}
/* With sds, it cleans the thread on join automatically. */
@@ -480,8 +473,7 @@ event_q_wake(ns_thrpool_t *tp)
}
static void
-event_q_notify(ns_job_t *job)
-{
+event_q_notify(ns_job_t *job) {
ns_thrpool_t *tp = job->tp;
/* if we are being called from a thread other than the
event loop thread, we have to notify that thread to
@@ -1133,6 +1125,11 @@ ns_job_rearm(ns_job_t *job)
PR_ASSERT(job);
pthread_mutex_lock(job->monitor);
PR_ASSERT(job->state == NS_JOB_WAITING || job->state == NS_JOB_RUNNING);
+
+ if (ns_thrpool_is_shutdown(job->tp)) {
+ return PR_FAILURE;
+ }
+
if (job->state == NS_JOB_WAITING) {
#ifdef DEBUG
ns_log(LOG_DEBUG, "ns_rearm_job %x state %d moving to
NS_JOB_NEEDS_ARM\n", job, job->state);
@@ -1494,6 +1491,7 @@ ns_thrpool_shutdown(struct ns_thrpool_t *tp)
__atomic_add_fetch(&(tp->shutdown), 1, __ATOMIC_SEQ_CST);
/* Wake up the idle worker threads so they can exit. */
+ /* Do we need this to be run in conjuction with our thread join loop incase threads
are still active? */
pthread_mutex_lock(&(tp->work_q_lock));
pthread_cond_broadcast(&(tp->work_q_cv));
pthread_mutex_unlock(&(tp->work_q_lock));
@@ -1510,6 +1508,12 @@ ns_thrpool_wait(ns_thrpool_t *tp)
while (sds_queue_dequeue(tp->thread_stack, (void **)&thr) == SDS_SUCCESS)
{
+ /* LAST CHANCE! Really make sure the thread workers are ready to go! */
+ /* In theory, they could still be blocked up here, but we hope not ... */
+ pthread_mutex_lock(&(tp->work_q_lock));
+ pthread_cond_broadcast(&(tp->work_q_cv));
+ pthread_mutex_unlock(&(tp->work_q_lock));
+
void *retval = NULL;
int32_t rc = pthread_join(thr->thr, &retval);
#ifdef DEBUG
diff --git a/src/nunc-stans/test/test_nuncstans_stress.h
b/src/nunc-stans/test/test_nuncstans_stress.h
new file mode 100644
index 0000000..6c19803
--- /dev/null
+++ b/src/nunc-stans/test/test_nuncstans_stress.h
@@ -0,0 +1,42 @@
+/** BEGIN COPYRIGHT BLOCK
+ * Copyright (c) 2017, Red Hat, Inc
+ * All rights reserved.
+ *
+ * License: GPL (version 3 or any later version).
+ * See LICENSE for details.
+ * END COPYRIGHT BLOCK **/
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+/* For cmocka */
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <nunc-stans.h>
+
+#include <stdio.h>
+#include <signal.h>
+
+#include <syslog.h>
+#include <string.h>
+#include <inttypes.h>
+
+#include <time.h>
+#include <sys/time.h>
+
+#include <assert.h>
+
+struct test_params {
+ int32_t client_thread_count;
+ int32_t server_thread_count;
+ int32_t jobs;
+ int32_t test_timeout;
+};
+
+int ns_stress_teardown(void **state);
+void ns_stress_test(void **state);
+
diff --git a/src/nunc-stans/test/test_nuncstans_stress.c
b/src/nunc-stans/test/test_nuncstans_stress_core.c
similarity index 92%
rename from src/nunc-stans/test/test_nuncstans_stress.c
rename to src/nunc-stans/test/test_nuncstans_stress_core.c
index 29585d5..ca4e45d 100644
--- a/src/nunc-stans/test/test_nuncstans_stress.c
+++ b/src/nunc-stans/test/test_nuncstans_stress_core.c
@@ -35,35 +35,8 @@
* removal, timers, and more.
*/
-#ifdef HAVE_CONFIG_H
-# include <config.h>
-#endif
-
-/* For cmocka */
-#include <stdarg.h>
-#include <stddef.h>
-#include <setjmp.h>
-#include <cmocka.h>
-
-/*
-#include <nspr.h>
-#include <plstr.h>
-#include <prlog.h>
-*/
-
-#include <nunc-stans.h>
-
-#include <stdio.h>
-#include <signal.h>
-
-#include <syslog.h>
-#include <string.h>
-#include <inttypes.h>
-
-#include <time.h>
-#include <sys/time.h>
-
-#include <assert.h>
+/* Our local stress test header */
+#include "test_nuncstans_stress.h"
struct conn_ctx {
size_t offset; /* current offset into buffer for reading or writing */
@@ -84,11 +57,13 @@ int32_t server_success_count = 0;
int32_t client_fail_count = 0;
int32_t client_timeout_count = 0;
int32_t server_fail_count = 0;
-int32_t job_count = 0;
-int32_t client_thread_count = 80;
-int32_t server_thread_count = 20;
-int32_t jobs = 200;
-int32_t test_timeout = 70;
+
+int
+ns_stress_teardown(void **state) {
+ struct test_params *tparams = (struct test_params *)*state;
+ free(tparams);
+ return 0;
+}
#define PR_WOULD_BLOCK(iii) (iii == PR_PENDING_INTERRUPT_ERROR) || (iii ==
PR_WOULD_BLOCK_ERROR)
@@ -320,7 +295,7 @@ static void
test_client_shutdown(struct ns_job_t *job)
{
do_logging(LOG_DEBUG, "Received shutdown signal\n");
- do_logging(LOG_DEBUG, "status .... job_count: %d fail_count: %d success_count:
%d\n", job_count, client_fail_count, client_success_count);
+ do_logging(LOG_DEBUG, "status .... fail_count: %d success_count: %d\n",
client_fail_count, client_success_count);
ns_thrpool_shutdown(ns_job_get_tp(job));
/* This also needs to start the thrpool shutdown for the server. */
ns_thrpool_shutdown(ns_job_get_data(job));
@@ -377,7 +352,9 @@ client_initiate_connection_cb(struct ns_job_t *job)
sock = PR_OpenTCPSocket(PR_AF_INET6);
if (sock == NULL) {
- do_logging(LOG_ERR, "Socket failed\n");
+ char *err = NULL;
+ PR_GetErrorText(err);
+ do_logging(LOG_ERR, "FAIL: Socket failed, %d -> %s\n",
PR_GetError(), err);
PR_AtomicAdd(&client_fail_count, 1);
goto done;
}
@@ -411,11 +388,13 @@ done:
static void
client_create_work(struct ns_job_t *job)
{
+ struct test_params *tparams = ns_job_get_data(job);
+
struct timespec ts;
PR_Sleep(PR_SecondsToInterval(1));
clock_gettime(CLOCK_MONOTONIC, &ts);
printf("BEGIN: %ld.%ld\n", ts.tv_sec, ts.tv_nsec);
- for (int32_t i = 0; i < jobs; i++) {
+ for (int32_t i = 0; i < tparams->jobs; i++) {
assert_int_equal(ns_add_job(ns_job_get_tp(job), NS_JOB_NONE|NS_JOB_THREAD,
client_initiate_connection_cb, NULL, NULL), 0);
}
assert_int_equal(ns_job_done(job), 0);
@@ -423,15 +402,17 @@ client_create_work(struct ns_job_t *job)
printf("Create work thread complete!\n");
}
-static void
-ns_stress_test(void **state __attribute__((unused)))
+void
+ns_stress_test(void **state)
{
+ struct test_params *tparams = *state;
+
/* Setup both thread pools. */
/* Client first */
- int32_t job_count = jobs * client_thread_count;
+ int32_t job_count = tparams->jobs * tparams->client_thread_count;
struct ns_thrpool_t *ctp;
struct ns_thrpool_config client_ns_config;
struct ns_job_t *sigterm_job = NULL;
@@ -442,12 +423,12 @@ ns_stress_test(void **state __attribute__((unused)))
struct ns_job_t *sigusr2_job = NULL;
struct ns_job_t *final_job = NULL;
- struct timeval timeout = { test_timeout, 0 };
+ struct timeval timeout = { tparams->test_timeout, 0 };
setup_logging();
ns_thrpool_config_init(&client_ns_config);
- client_ns_config.max_threads = client_thread_count;
+ client_ns_config.max_threads = tparams->client_thread_count;
client_ns_config.log_fct = do_vlogging;
ctp = ns_thrpool_new(&client_ns_config);
@@ -458,7 +439,7 @@ ns_stress_test(void **state __attribute__((unused)))
struct ns_job_t *listen_job = NULL;
ns_thrpool_config_init(&server_ns_config);
- server_ns_config.max_threads = server_thread_count;
+ server_ns_config.max_threads = tparams->server_thread_count;
server_ns_config.log_fct = do_vlogging;
stp = ns_thrpool_new(&server_ns_config);
@@ -493,8 +474,8 @@ ns_stress_test(void **state __attribute__((unused)))
assert_int_equal(ns_add_timeout_job(ctp, &timeout, NS_JOB_NONE|NS_JOB_THREAD,
test_client_shutdown, stp, &final_job), 0);
/* While true, add connect / write jobs */
- for (PRInt32 i = 0; i < client_thread_count; i++) {
- assert_int_equal(ns_add_job(ctp, NS_JOB_NONE|NS_JOB_THREAD, client_create_work,
NULL, NULL), 0);
+ for (PRInt32 i = 0; i < tparams->client_thread_count; i++) {
+ assert_int_equal(ns_add_job(ctp, NS_JOB_NONE|NS_JOB_THREAD, client_create_work,
tparams, NULL), 0);
}
/* Wait for all the clients to be done dispatching jobs to the server */
@@ -542,17 +523,9 @@ ns_stress_test(void **state __attribute__((unused)))
assert_int_equal(client_success_count, job_count);
*/
assert_int_equal(server_success_count, client_success_count);
+ int32_t job_threshold = (tparams->jobs * tparams->client_thread_count) * 0.95;
+ assert_true(client_success_count >= job_threshold);
PR_Cleanup();
}
-
-int
-main (void)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(ns_stress_test),
- };
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
-
diff --git a/src/nunc-stans/test/test_nuncstans_stress_large.c
b/src/nunc-stans/test/test_nuncstans_stress_large.c
new file mode 100644
index 0000000..a3e0d5e
--- /dev/null
+++ b/src/nunc-stans/test/test_nuncstans_stress_large.c
@@ -0,0 +1,33 @@
+/** BEGIN COPYRIGHT BLOCK
+ * Copyright (c) 2017, Red Hat, Inc
+ * All rights reserved.
+ *
+ * License: GPL (version 3 or any later version).
+ * See LICENSE for details.
+ * END COPYRIGHT BLOCK **/
+
+#include "test_nuncstans_stress.h"
+
+int
+ns_stress_large_setup(void **state) {
+ struct test_params *tparams = malloc(sizeof(struct test_params));
+ tparams->client_thread_count = 80;
+ tparams->server_thread_count = 20;
+ tparams->jobs = 200;
+ tparams->test_timeout = 70;
+ *state = tparams;
+ return 0;
+}
+
+int
+main (void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test_setup_teardown(ns_stress_test,
+ ns_stress_large_setup,
+ ns_stress_teardown),
+ };
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+
diff --git a/src/nunc-stans/test/test_nuncstans_stress_small.c
b/src/nunc-stans/test/test_nuncstans_stress_small.c
new file mode 100644
index 0000000..da57ab0
--- /dev/null
+++ b/src/nunc-stans/test/test_nuncstans_stress_small.c
@@ -0,0 +1,33 @@
+/** BEGIN COPYRIGHT BLOCK
+ * Copyright (c) 2017, Red Hat, Inc
+ * All rights reserved.
+ *
+ * License: GPL (version 3 or any later version).
+ * See LICENSE for details.
+ * END COPYRIGHT BLOCK **/
+
+#include "test_nuncstans_stress.h"
+
+int
+ns_stress_small_setup(void **state) {
+ struct test_params *tparams = malloc(sizeof(struct test_params));
+ tparams->client_thread_count = 4;
+ tparams->server_thread_count = 1;
+ tparams->jobs = 64;
+ tparams->test_timeout = 30;
+ *state = tparams;
+ return 0;
+}
+
+int
+main (void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test_setup_teardown(ns_stress_test,
+ ns_stress_small_setup,
+ ns_stress_teardown),
+ };
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.