From 584d3039baf32d047d0f70df1f1cd40e794287ac Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Tue, 26 Aug 2014 18:20:59 +0200 Subject: [PATCH 01/10] dcb_remove_callback() still had the old callback prototype --- server/core/dcb.c | 2 +- server/include/dcb.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 1dfde0b58..d0d93aff8 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1771,7 +1771,7 @@ int rval = 1; * @return Non-zero (true) if the callback was removed */ int -dcb_remove_callback(DCB *dcb, DCB_REASON reason, int (*callback)(struct dcb *, DCB_REASON), void *userdata) +dcb_remove_callback(DCB *dcb, DCB_REASON reason, int (*callback)(struct dcb *, DCB_REASON, void *), void *userdata) { DCB_CALLBACK *cb, *pcb = NULL; int rval = 0; diff --git a/server/include/dcb.h b/server/include/dcb.h index 88964fc31..956bb3ef6 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -289,7 +289,7 @@ void dcb_hashtable_stats(DCB *, void *); /**< Print statisitics */ void dcb_add_to_zombieslist(DCB* dcb); int dcb_add_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, void *), void *); -int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON), +int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, void *), void *); int dcb_isvalid(DCB *); /* Check the DCB is in the linked list */ From 25c5d1af25cb6d3df4fe2034f4fdca6f1ade7477 Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Tue, 26 Aug 2014 18:25:28 +0200 Subject: [PATCH 02/10] fixed some warnings about const -> non-const conversions --- log_manager/log_manager.cc | 12 ++++++------ log_manager/log_manager.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index c8f96266e..4138fdcb3 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -249,7 +249,7 @@ static int logmanager_write_log( bool use_valist, bool spread_down, size_t len, - char* str, + const char* str, va_list valist); static blockbuf_t* blockbuf_init(logfile_id_t id); @@ -603,7 +603,7 @@ static int logmanager_write_log( bool use_valist, bool spread_down, size_t str_len, - char* str, + const char* str, va_list valist) { logfile_t* lf; @@ -617,7 +617,7 @@ static int logmanager_write_log( CHK_LOGMANAGER(lm); if (id < LOGFILE_FIRST || id > LOGFILE_LAST) { - char* errstr = "Invalid logfile id argument."; + const char* errstr = "Invalid logfile id argument."; /** * invalid id, since we don't have logfile yet. */ @@ -1092,7 +1092,7 @@ static bool logfile_set_enabled( CHK_LOGMANAGER(lm); if (id < LOGFILE_FIRST || id > LOGFILE_LAST) { - char* errstr = "Invalid logfile id argument."; + const char* errstr = "Invalid logfile id argument."; /** * invalid id, since we don't have logfile yet. */ @@ -1146,7 +1146,7 @@ return_succp: int skygw_log_write_flush( logfile_id_t id, - char* str, + const char* str, ...) { int err = 0; @@ -1199,7 +1199,7 @@ return_err: int skygw_log_write( logfile_id_t id, - char* str, + const char* str, ...) { int err = 0; diff --git a/log_manager/log_manager.h b/log_manager/log_manager.h index b96669fa2..b3dc73f18 100644 --- a/log_manager/log_manager.h +++ b/log_manager/log_manager.h @@ -66,9 +66,9 @@ void skygw_logmanager_exit(void); * free private write buffer list */ void skygw_log_done(void); -int skygw_log_write(logfile_id_t id, char* format, ...); +int skygw_log_write(logfile_id_t id, const char* format, ...); int skygw_log_flush(logfile_id_t id); -int skygw_log_write_flush(logfile_id_t id, char* format, ...); +int skygw_log_write_flush(logfile_id_t id, const char* format, ...); int skygw_log_enable(logfile_id_t id); int skygw_log_disable(logfile_id_t id); From 6a03976e4f63a4a7260a1efaee3d61ed37ffbeba Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Wed, 3 Sep 2014 01:03:10 +0200 Subject: [PATCH 03/10] support --long options with getopt_long() --- client/Makefile | 2 +- client/maxadmin.c | 165 ++++++++++++++++-------------------------- server/core/gateway.c | 42 +++++++++-- 3 files changed, 96 insertions(+), 113 deletions(-) diff --git a/client/Makefile b/client/Makefile index 19f38785f..22220db2d 100644 --- a/client/Makefile +++ b/client/Makefile @@ -33,7 +33,7 @@ endif CC=cc -CFLAGS=-c -Wall -g $(HISTFLAG) +CFLAGS=-c -Wall -g $(HISTFLAG) -I ../server/include SRCS= maxadmin.c diff --git a/client/maxadmin.c b/client/maxadmin.c index 5710cc35a..3c5b55490 100644 --- a/client/maxadmin.c +++ b/client/maxadmin.c @@ -47,6 +47,9 @@ #include #include #include +#include + +#include #ifdef HISTORY #include @@ -57,7 +60,8 @@ static int setipaddress(struct in_addr *a, char *p); static int authMaxScale(int so, char *user, char *password); static int sendCommand(int so, char *cmd); static void DoSource(int so, char *cmd); -static void DoUsage(); +static void PrintVersion(const char *progname); +static void DoUsage(const char *progname); #ifdef HISTORY static char * @@ -69,6 +73,16 @@ prompt(EditLine *el __attribute__((__unused__))) } #endif +static struct option long_options[] = { + {"host", required_argument, 0, 'h'}, + {"user", required_argument, 0, 'u'}, + {"password", required_argument, 0, 'p'}, + {"port", required_argument, 0, 'P'}, + {"version", no_argument, 0, 'v'}, + {"help", no_argument, 0, '?'}, + {0, 0, 0, 0} +}; + /** * The main for the maxadmin client * @@ -78,7 +92,7 @@ prompt(EditLine *el __attribute__((__unused__))) int main(int argc, char **argv) { -int i, num, rv, fatal = 0; +int i, num, rv; #ifdef HISTORY char *buf; EditLine *el = NULL; @@ -95,107 +109,39 @@ char *user = "admin"; char *passwd = NULL; int so, cmdlen; char *cmd; -int argno = 0; +int option_index = 0; +char c; cmd = malloc(1); *cmd = 0; cmdlen = 1; - for (i = 1; i < argc; i++) - { - if (argv[i][0] == '-') - { - switch (argv[i][1]) - { - case 'u': /* User */ - if (argv[i][2]) - user = &(argv[i][2]); - else if (i + 1 < argc) - user = argv[++i]; - else - { - fprintf(stderr, "Missing username" - "in -u option.\n"); - fatal = 1; - } - break; - case 'p': /* Password */ - if (argv[i][2]) - passwd = &(argv[i][2]); - else if (i + 1 < argc) - passwd = argv[++i]; - else - { - fprintf(stderr, "Missing password " - "in -p option.\n"); - fatal = 1; - } - break; - case 'h': /* hostname */ - if (argv[i][2]) - hostname = &(argv[i][2]); - else if (i + 1 < argc) - hostname = argv[++i]; - else - { - fprintf(stderr, "Missing hostname value " - "in -h option.\n"); - fatal = 1; - } - break; - case 'P': /* Port */ - if (argv[i][2]) - port = &(argv[i][2]); - else if (i + 1 < argc) - port = argv[++i]; - else - { - fprintf(stderr, "Missing Port value " - "in -P option.\n"); - fatal = 1; - } - break; - case '-': - { - char *word; - - word = &argv[i][2]; - if (strcmp(word, "help") == 0) - { - DoUsage(); - exit(0); - } - break; - } - } - } - else - { - /* Arguments after the second argument are quoted - * to allow for quoted names on the command line - * to be passed on in quotes. - */ - if (argno++ > 1) - { - cmdlen += strlen(argv[i]) + 3; - cmd = realloc(cmd, cmdlen); - strcat(cmd, "\""); - strcat(cmd, argv[i]); - strcat(cmd, "\" "); - } - else - { - cmdlen += strlen(argv[i]) + 1; - cmd = realloc(cmd, cmdlen); - strcat(cmd, argv[i]); - strcat(cmd, " "); - } - } + while ((c = getopt_long(argc, argv, "h:p:P:u:v?", + long_options, &option_index)) + >= 0) + { + switch (c) { + case 'h': + hostname = strdup(optarg); + break; + case 'p': + passwd = strdup(optarg); + break; + case 'P': + port = strdup(optarg); + break; + case 'u': + user = strdup(optarg); + break; + case 'v': + PrintVersion(*argv); + exit(EXIT_SUCCESS); + case '?': + DoUsage(*argv); + exit(optopt ? EXIT_FAILURE : EXIT_SUCCESS); + } } - if (fatal) - exit(1); - if (passwd == NULL) { struct termios tty_attr; @@ -532,23 +478,34 @@ FILE *fp; return; } +/** + * Print version information + */ +static void +PrintVersion(const char *progname) +{ + printf("%s Version %s\n", progname, MAXSCALE_VERSION); +} + /** * Display the --help text. */ static void -DoUsage() +DoUsage(const char *progname) { - printf("maxadmin: The MaxScale administrative and monitor client.\n\n"); - printf("Usage: maxadmin [-u user] [-p password] [-h hostname] [-P port] [ | ]\n\n"); - printf(" -u user The user name to use for the connection, default\n"); + PrintVersion(progname); + printf("The MaxScale administrative and monitor client.\n\n"); + printf("Usage: %s [-u user] [-p password] [-h hostname] [-P port] [ | ]\n\n", progname); + printf(" -u|--user=... The user name to use for the connection, default\n"); printf(" is admin.\n"); - printf(" -p password The user password, if not given the password will\n"); + printf(" -p|--password=... The user password, if not given the password will\n"); printf(" be prompted for interactively\n"); - printf(" -h hostname The maxscale host to connecto to. The default is\n"); + printf(" -h|--hostname=... The maxscale host to connecto to. The default is\n"); printf(" localhost\n"); - printf(" -P port The port to use for the connection, the default\n"); + printf(" -P|--port=... The port to use for the connection, the default\n"); printf(" port is 6603.\n"); - printf(" --help Print this help text.\n"); + printf(" -v|--version print version information and exit\n"); + printf(" -?|--help Print this help text.\n"); printf("Any remaining arguments are treated as MaxScale commands or a file\n"); printf("containing commands to execute.\n"); } diff --git a/server/core/gateway.c b/server/core/gateway.c index fda19fff8..bb3fe9485 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -128,6 +129,16 @@ static bool libmysqld_started = FALSE; */ static bool daemon_mode = true; +const char *progname = NULL; +static struct option long_options[] = { + {"homedir", required_argument, 0, 'c'}, + {"config", required_argument, 0, 'f'}, + {"nodeamon", required_argument, 0, 'd'}, + {"version", no_argument, 0, 'v'}, + {"help", no_argument, 0, '?'}, + {0, 0, 0, 0} +}; + static void log_flush_shutdown(void); static void log_flush_cb(void* arg); static int write_pid_file(char *); /* write MaxScale pidfile */ @@ -828,15 +839,17 @@ return_cnf_file_buf: return cnf_file_buf; } - static void usage(void) { fprintf(stderr, - "*\n* Usage : maxscale [-h] | [-d] [-c ] [-f ]\n* where:\n* " - "-h help\n* -d enable running in terminal process (default:disabled)\n* " - "-c relative|absolute MaxScale home directory\n* " - "-f relative|absolute pathname of MaxScale configuration file (default:MAXSCALE_HOME/etc/MaxScale.cnf)\n*\n"); + "\nUsage : %s [-h] | [-d] [-c ] [-f ]\n\n" + " -d|--nodaemon enable running in terminal process (default:disabled)\n" + " -c|--homedir=... relative|absolute MaxScale home directory\n" + " -f|--config=... relative|absolute pathname of MaxScale configuration file\n" + " (default: $MAXSCALE_HOME/etc/MaxScale.cnf)\n" + " -v|--version print version info and exit\n" + " -?|--help show this help\n" + , progname); } /** @@ -893,6 +906,7 @@ int main(int argc, char **argv) char* cnf_file_path = NULL; /*< conf file, to be freed */ char* cnf_file_arg = NULL; /*< conf filename from cmd-line arg */ void* log_flush_thr = NULL; + int option_index; ssize_t log_flush_timeout_ms = 0; sigset_t sigset; sigset_t sigpipe_mask; @@ -904,6 +918,8 @@ int main(int argc, char **argv) sigemptyset(&sigpipe_mask); sigaddset(&sigpipe_mask, SIGPIPE); + progname = *argv; + #if defined(SS_DEBUG) memset(conn_open, 0, sizeof(bool)*10240); memset(dcb_fake_write_errno, 0, sizeof(unsigned char)*10240); @@ -930,7 +946,8 @@ int main(int argc, char **argv) goto return_main; } } - while ((opt = getopt(argc, argv, "dc:f:h")) != -1) + while ((opt = getopt_long(argc, argv, "dc:f:v?", + long_options, &option_index)) != -1) { bool succp = true; @@ -1011,9 +1028,18 @@ int main(int argc, char **argv) succp = false; } break; + + case 'v': + rc = EXIT_SUCCESS; + goto return_main; + + case '?': + usage(); + rc = EXIT_SUCCESS; + goto return_main; default: - usage(); + usage(); succp = false; break; } From 675baad4b132fc69994e2b32ea5f8f3886d34368 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Wed, 17 Sep 2014 14:54:40 +0100 Subject: [PATCH 04/10] Fixed typo --- server/core/modutil.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index e2800b849..7247f181d 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -125,7 +125,7 @@ unsigned char *ptr; /** * Replace the contents of a GWBUF with the new SQL statement passed as a text string. * The routine takes care of the modification needed to the MySQL packet, - * returning a GWBUF chian that cna be used to send the data to a MySQL server + * returning a GWBUF chain that can be used to send the data to a MySQL server * * @param orig The original request in a GWBUF * @param sql The SQL text to replace in the packet @@ -225,4 +225,4 @@ char* modutil_get_query( } /*< switch */ retblock: return query_str; -} \ No newline at end of file +} From 2402d55de6e48e468339f7d093ee8a3ebf8f7eaa Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Fri, 19 Sep 2014 10:50:54 +0100 Subject: [PATCH 05/10] Some general tidyup plus addition of code to block zombie processing if epoll_wait returned multiple descriptors --- server/core/dcb.c | 153 +++++++++++++++++++++++--------------- server/core/housekeeper.c | 1 + server/core/poll.c | 23 ++++-- server/include/dcb.h | 4 +- 4 files changed, 112 insertions(+), 69 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index a1681caa7..b8f9108de 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -403,13 +403,16 @@ DCB_CALLBACK *cb; * operation of clearing this bit means that no bits are set in * the memdata.bitmask then the DCB is no longer able to be * referenced and it can be finally removed. - * Thread won't clear its bit from bitmask of the DCB it is still using. + * + * The excluded DCB allows a thread to exclude a DCB from zombie processing. + * It is used when a thread calls dcb_process_zombies when there is + * a DCB that the caller knows it will continue processing with. * * @param threadid The thread ID of the caller - * @param dcb_in_use The DCB the thread currently uses, NULL or valid DCB. + * @param excluded The DCB the thread currently uses, NULL or valid DCB. */ DCB * -dcb_process_zombies(int threadid, DCB *dcb_in_use) +dcb_process_zombies(int threadid, DCB *excluded) { DCB *ptr, *lptr; DCB* dcb_list = NULL; @@ -426,78 +429,100 @@ bool succp = false; if (!zombies) return NULL; + /* + * Process the zombie queue and create a list of DCB's that can be + * finally freed. This processing is down under a spinlock that + * will prevent new entries being added to the zombie queue. Therefore + * we do not want to do any expensive operations under this spinlock + * as it will block other threads. The expensive operations will be + * performed on the victim queue within holding the zombie queue + * spinlock. + */ spinlock_acquire(&zombiespin); ptr = zombies; lptr = NULL; while (ptr) { CHK_DCB(ptr); - /** Don't clear the bit from DCB the user currently uses */ - if (dcb_in_use == NULL || ptr != dcb_in_use) - { - bitmask_clear(&ptr->memdata.bitmask, threadid); - } - if (ptr == dcb_in_use) - ss_dassert(!bitmask_isallclear(&ptr->memdata.bitmask)); - - if (bitmask_isallclear(&ptr->memdata.bitmask)) - { - /** - * Remove the DCB from the zombie queue - * and call the final free routine for the - * DCB - * - * ptr is the DCB we are processing - * lptr is the previous DCB on the zombie queue - * or NULL if the DCB is at the head of the queue - * tptr is the DCB after the one we are processing - * on the zombie queue - */ - DCB *tptr = ptr->memdata.next; - if (lptr == NULL) - zombies = tptr; - else - lptr->memdata.next = tptr; - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [dcb_process_zombies] Remove dcb %p fd %d " - "in state %s from zombies list.", - pthread_self(), - ptr, - ptr->fd, - STRDCBSTATE(ptr->state)))); - ss_info_dassert(ptr->state == DCB_STATE_ZOMBIE, - "dcb not in DCB_STATE_ZOMBIE state."); - /*< - * Move dcb to linked list of victim dcbs. - */ - if (dcb_list == NULL) { - dcb_list = ptr; - dcb = dcb_list; - } else { - dcb->memdata.next = ptr; - dcb = dcb->memdata.next; - } - dcb->memdata.next = NULL; - ptr = tptr; - } - else + + /* + * Skip processing of the excluded DCB + */ + if (ptr == excluded) { lptr = ptr; ptr = ptr->memdata.next; } + else + { + + bitmask_clear(&ptr->memdata.bitmask, threadid); + + if (bitmask_isallclear(&ptr->memdata.bitmask)) + { + /** + * Remove the DCB from the zombie queue + * and call the final free routine for the + * DCB + * + * ptr is the DCB we are processing + * lptr is the previous DCB on the zombie queue + * or NULL if the DCB is at the head of the + * queue tptr is the DCB after the one we are + * processing on the zombie queue + */ + DCB *tptr = ptr->memdata.next; + if (lptr == NULL) + zombies = tptr; + else + lptr->memdata.next = tptr; + LOGIF(LD, (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [dcb_process_zombies] Remove dcb " + "%p fd %d " "in state %s from the " + "list of zombies.", + pthread_self(), + ptr, + ptr->fd, + STRDCBSTATE(ptr->state)))); + ss_info_dassert(ptr->state == DCB_STATE_ZOMBIE, + "dcb not in DCB_STATE_ZOMBIE state."); + /*< + * Move dcb to linked list of victim dcbs. + */ + if (dcb_list == NULL) { + dcb_list = ptr; + dcb = dcb_list; + } else { + dcb->memdata.next = ptr; + dcb = dcb->memdata.next; + } + dcb->memdata.next = NULL; + ptr = tptr; + } + else + { + lptr = ptr; + ptr = ptr->memdata.next; + } + } } spinlock_release(&zombiespin); + /* + * Process the victim queue. These are DCBs that are not in + * use by any thread. + * The corresponding file descriptor is closed, the DCB marked + * as disconnected and the DCB itself is fianlly freed. + */ dcb = dcb_list; - /** Close, and set DISCONNECTED victims */ while (dcb != NULL) { DCB* dcb_next = NULL; int rc = 0; /*< * Close file descriptor and move to clean-up phase. */ - ss_dassert(dcb_in_use != dcb); + ss_dassert(excluded != dcb); rc = close(dcb->fd); if (rc < 0) { @@ -1119,8 +1144,8 @@ int above_water; /** * Removes dcb from poll set, and adds it to zombies list. As a consequense, * dcb first moves to DCB_STATE_NOPOLLING, and then to DCB_STATE_ZOMBIE state. - * At the end of the function state may not be DCB_STATE_ZOMBIE because once dcb_initlock - * is released parallel threads may change the state. + * At the end of the function state may not be DCB_STATE_ZOMBIE because once + * dcb_initlock is released parallel threads may change the state. * * Parameters: * @param dcb The DCB to close @@ -1943,9 +1968,11 @@ int rval = 0; * to return immediately and and process other events. * * @param dcb The DCB that has data available + * @param thread_id The ID of the calling thread + * @param nozombies If non-zero then do not do zombie processing */ void -dcb_pollin(DCB *dcb, int thread_id) +dcb_pollin(DCB *dcb, int thread_id, int nozombies) { spinlock_acquire(&dcb->pollinlock); @@ -1956,7 +1983,8 @@ dcb_pollin(DCB *dcb, int thread_id) if (dcb->readcheck) { dcb->stats.n_readrechecks++; - dcb_process_zombies(thread_id, dcb); + if (!nozombies) + dcb_process_zombies(thread_id, dcb); } dcb->readcheck = 0; spinlock_release(&dcb->pollinlock); @@ -1987,9 +2015,11 @@ dcb_pollin(DCB *dcb, int thread_id) * to return immediately and and process other events. * * @param dcb The DCB thats available for writes + * @param thread_id The ID of the calling thread + * @param nozombies If non-zero then do not do zombie processing */ void -dcb_pollout(DCB *dcb, int thread_id) +dcb_pollout(DCB *dcb, int thread_id, int nozombies) { spinlock_acquire(&dcb->polloutlock); @@ -1999,7 +2029,8 @@ dcb_pollout(DCB *dcb, int thread_id) do { if (dcb->writecheck) { - dcb_process_zombies(thread_id, dcb); + if (!nozombies) + dcb_process_zombies(thread_id, dcb); dcb->stats.n_writerechecks++; } dcb->writecheck = 0; diff --git a/server/core/housekeeper.c b/server/core/housekeeper.c index 6180f24a5..5aa5c7a29 100644 --- a/server/core/housekeeper.c +++ b/server/core/housekeeper.c @@ -16,6 +16,7 @@ * Copyright SkySQL Ab 2014 */ #include +#include #include #include #include diff --git a/server/core/poll.c b/server/core/poll.c index e2b284e27..b3a0e6b70 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -49,10 +49,19 @@ extern int lm_enabled_logfiles_bitmask; * @endverbatim */ +/** + * Control the use of mutexes for the epoll_wait call. Setting to 1 will + * cause the epoll_wait calls to be moved under a mutex. This may be useful + * for debuggign purposes but should be avoided in general use. + */ +#define MUTEX_EPOLL 0 + static int epoll_fd = -1; /*< The epoll file descriptor */ static int do_shutdown = 0; /*< Flag the shutdown of the poll subsystem */ static GWBITMASK poll_mask; +#if MUTEX_EPOLL static simple_mutex_t epoll_wait_mutex; /*< serializes calls to epoll_wait */ +#endif static int n_waiting = 0; /*< No. of threads in epoll_wait */ /** @@ -154,7 +163,9 @@ int i; thread_data[i].state = THREAD_STOPPED; } } +#if MUTEX_EPOLL simple_mutex_init(&epoll_wait_mutex, "epoll_wait_mutex"); +#endif hktask_add("Load Average", poll_loadav, NULL, POLL_LOAD_FREQ); n_avg_samples = 15 * 60 / POLL_LOAD_FREQ; @@ -359,7 +370,7 @@ DCB *zombies = NULL; thread_id))); no_op = TRUE; } -#if 0 +#if MUTEX_EPOLL simple_mutex_lock(&epoll_wait_mutex, TRUE); #endif if (thread_data) @@ -385,7 +396,7 @@ DCB *zombies = NULL; { atomic_add(&n_waiting, -1); if (process_zombies_only) { -#if 0 +#if MUTEX_EPOLL simple_mutex_unlock(&epoll_wait_mutex); #endif goto process_zombies; @@ -413,7 +424,7 @@ DCB *zombies = NULL; if (n_waiting == 0) atomic_add(&pollStats.n_nothreads, 1); -#if 0 +#if MUTEX_EPOLL simple_mutex_unlock(&epoll_wait_mutex); #endif #endif /* BLOCKINGPOLL */ @@ -442,7 +453,7 @@ DCB *zombies = NULL; for (i = 0; i < nfds; i++) { - DCB *dcb = (DCB *)events[i].data.ptr; + DCB *dcb = (DCB *)events[i].data.ptr; __uint32_t ev = events[i].events; CHK_DCB(dcb); @@ -504,7 +515,7 @@ DCB *zombies = NULL; #else atomic_add(&pollStats.n_write, 1); - dcb_pollout(dcb, thread_id); + dcb_pollout(dcb, thread_id, nfds); #endif } else { LOGIF(LD, (skygw_log_write( @@ -554,7 +565,7 @@ DCB *zombies = NULL; #if MUTEX_BLOCK dcb->func.read(dcb); #else - dcb_pollin(dcb, thread_id); + dcb_pollin(dcb, thread_id, nfds); #endif } #if MUTEX_BLOCK diff --git a/server/include/dcb.h b/server/include/dcb.h index 17cc0c998..3966dfead 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -271,8 +271,8 @@ int fail_accept_errno; #define DCB_BELOW_LOW_WATER(x) ((x)->low_water && (x)->writeqlen < (x)->low_water) #define DCB_ABOVE_HIGH_WATER(x) ((x)->high_water && (x)->writeqlen > (x)->high_water) -void dcb_pollin(DCB *, int); -void dcb_pollout(DCB *, int); +void dcb_pollin(DCB *, int, int); +void dcb_pollout(DCB *, int, int); DCB *dcb_get_zombies(void); int gw_write( #if defined(SS_DEBUG) From 2d2fc28b074ef8c10aa6c0347ed4f6bb15632a04 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Fri, 19 Sep 2014 11:40:16 +0100 Subject: [PATCH 06/10] Addition of code to prevent multiple hangup's beign processed on the same DCB --- server/core/poll.c | 20 ++++++++++++++++++-- server/include/dcb.h | 7 +++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/server/core/poll.c b/server/core/poll.c index b3a0e6b70..ff8cfe7b3 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -620,7 +620,15 @@ DCB *zombies = NULL; eno, strerror(eno)))); atomic_add(&pollStats.n_hup, 1); - dcb->func.hangup(dcb); + spinlock_acquire(&dcb->dcb_initlock); + if ((dcb->flags & DCBF_HUNG) == 0) + { + dcb->flags |= DCBF_HUNG; + spinlock_release(&dcb->dcb_initlock); + dcb->func.hangup(dcb); + } + else + spinlock_release(&dcb->dcb_initlock); } if (ev & EPOLLRDHUP) @@ -639,7 +647,15 @@ DCB *zombies = NULL; eno, strerror(eno)))); atomic_add(&pollStats.n_hup, 1); - dcb->func.hangup(dcb); + spinlock_acquire(&dcb->dcb_initlock); + if ((dcb->flags & DCBF_HUNG) == 0) + { + dcb->flags |= DCBF_HUNG; + spinlock_release(&dcb->dcb_initlock); + dcb->func.hangup(dcb); + } + else + spinlock_release(&dcb->dcb_initlock); } } /*< for */ no_op = FALSE; diff --git a/server/include/dcb.h b/server/include/dcb.h index 3966dfead..06687f349 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -317,6 +317,9 @@ void dcb_call_foreach (DCB_REASON reason); void dcb_call_foreach ( DCB_REASON reason); -/* DCB flags values */ -#define DCBF_CLONE 0x0001 /* DCB is a clone */ +/** + * DCB flags values + */ +#define DCBF_CLONE 0x0001 /*< DCB is a clone */ +#define DCBF_HUNG 0x0002 /*< Hangup has been dispatched */ #endif /* _DCB_H */ From c4fb3e490bde03a6fdaf948993c0af5549ace325 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Fri, 19 Sep 2014 15:52:44 +0100 Subject: [PATCH 07/10] Bug 546 - use of weghtby causes entry in error log --- server/core/config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/config.c b/server/core/config.c index 1ba2bcbf4..995e0e0f4 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -1575,6 +1575,7 @@ static char *service_params[] = "use_sql_variables_in", /*< rwsplit only */ "version_string", "filters", + "weightby", NULL }; From 503b942b5cbd58928c1dbf5d5a992fa4e9798e21 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Fri, 19 Sep 2014 18:24:11 +0100 Subject: [PATCH 08/10] bug 506 - add command line option (-l --log=file) to log to file or shared memory. Only affects trace and debug logs. --- server/core/gateway.c | 47 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index e42397acb..efce2d97a 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -137,6 +137,7 @@ static struct option long_options[] = { {"homedir", required_argument, 0, 'c'}, {"config", required_argument, 0, 'f'}, {"nodeamon", required_argument, 0, 'd'}, + {"log", required_argument, 0, 'l'}, {"version", no_argument, 0, 'v'}, {"help", no_argument, 0, '?'}, {0, 0, 0, 0} @@ -897,6 +898,8 @@ static void usage(void) " -c|--homedir=... relative|absolute MaxScale home directory\n" " -f|--config=... relative|absolute pathname of MaxScale configuration file\n" " (default: $MAXSCALE_HOME/etc/MaxScale.cnf)\n" + " -l|--log=... log to file or shared memory\n" + " -lfile or -lshm - defaults to shared memory\n" " -v|--version print version info and exit\n" " -?|--help show this help\n" , progname); @@ -957,6 +960,7 @@ int main(int argc, char **argv) char* cnf_file_arg = NULL; /*< conf filename from cmd-line arg */ void* log_flush_thr = NULL; int option_index; + int logtofile = 0; /* Use shared memory or file */ ssize_t log_flush_timeout_ms = 0; sigset_t sigset; sigset_t sigpipe_mask; @@ -996,7 +1000,7 @@ int main(int argc, char **argv) goto return_main; } } - while ((opt = getopt_long(argc, argv, "dc:f:v?", + while ((opt = getopt_long(argc, argv, "dc:f:l:v?", long_options, &option_index)) != -1) { bool succp = true; @@ -1082,6 +1086,24 @@ int main(int argc, char **argv) case 'v': rc = EXIT_SUCCESS; goto return_main; + + case 'l': + if (strncasecmp(optarg, "file") == 0) + logtofile = 1; + else if (strncasecmp(optarg, "shm") == 0) + logtofile = 0; + else + { + char* logerr = "Configuration file argument " + "identifier \'-l\' was specified but " + "the argument didn't specify\n a valid " + "configuration file or the argument " + "was missing."; + print_log_n_stderr(true, true, logerr, logerr, 0); + usage(); + succp = false; + } + break; case '?': usage(); @@ -1371,12 +1393,23 @@ int main(int argc, char **argv) argv[0] = "MaxScale"; argv[1] = "-j"; argv[2] = buf; - argv[3] = "-s"; /*< store to shared memory */ - argv[4] = "LOGFILE_DEBUG,LOGFILE_TRACE"; /*< ..these logs to shm */ - argv[5] = "-l"; /*< write to syslog */ - argv[6] = "LOGFILE_MESSAGE,LOGFILE_ERROR"; /*< ..these logs to syslog */ - argv[7] = NULL; - skygw_logmanager_init(7, argv); + if (logtofile) + { + argv[3] = "-l"; /*< write to syslog */ + argv[4] = "LOGFILE_MESSAGE,LOGFILE_ERROR" + "LOGFILE_DEBUG,LOGFILE_TRACE"; + argv[5] = NULL; + skygw_logmanager_init(5, argv); + } + else + { + argv[3] = "-s"; /*< store to shared memory */ + argv[4] = "LOGFILE_DEBUG,LOGFILE_TRACE"; /*< ..these logs to shm */ + argv[5] = "-l"; /*< write to syslog */ + argv[6] = "LOGFILE_MESSAGE,LOGFILE_ERROR"; /*< ..these logs to syslog */ + argv[7] = NULL; + skygw_logmanager_init(7, argv); + } } /*< From bf0d41674d2e2a092ba243989c944c16fa1784ce Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 22 Sep 2014 11:34:39 +0300 Subject: [PATCH 09/10] Droppped CMake requirement to 2.6 --- CMakeLists.txt | 2 +- README | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c99491617..07b4f1af6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12) +cmake_minimum_required(VERSION 2.6) include(macros.cmake) diff --git a/README b/README index 33bd60ba1..6f8c2dd0a 100644 --- a/README +++ b/README @@ -157,7 +157,7 @@ Please check errmsg.sys is found in the MaxScale install_dir DEST/MaxScale/mysql You can also build MaxScale with CMake which makes the build process a bit more simple. All the same dependencies are required as with the normal MaxScale build with the addition of CMake -version 2.6 for regular builds and 2.8 or newer if you wish to generate packages. +version 2.6 for regular builds and 2.8.12 or newer if you wish to generate packages. CMake tries to find all the required directories and files on its own but if it can't find them or you wish to explicitly state the locations you can pass additional options to CMake by using the -D flag. To confirm the variable @@ -169,13 +169,19 @@ makes it easy to get rid of everything you built. By default, MaxScale installs to /usr/local/skysql and places init.d scripts and ldconfig files into their folders. Change the INSTALL_DIR variable to your desired installation directory and set INSTALL_SYSTEM_FILES=N to prevent the init.d script and ldconfig file installation. -To build and install MaxScale using CMake with a custom install location and a separate build directory: +To build MaxScale using CMake: - cmake -D_INSTALL_DIR= - - make - - make install + cd + + mkdir build + + cd build + + cmake .. + + make + + make install This generates the required makefiles in the current directory, compiles and links all the programs and installs all the required files in their right places. @@ -198,6 +204,7 @@ GCOV=[Y|N] Generate gcov output BUILD_TESTS=[Y|N] Build tests DEBUG_OUTPUT=[Y|N] Produce debugging output when configuring CMake + \section Running Running MaxScale MaxScale consists of a core executable and a number of modules that implement From fc848665e567cba30ed29804af0a7226e5edcebe Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 22 Sep 2014 13:14:53 +0300 Subject: [PATCH 10/10] Added variables for RabbitMQ headers and libraries, added more error checks. modified: CMakeLists.txt modified: README modified: macros.cmake modified: query_classifier/test/canonical_tests/CMakeLists.txt modified: rabbitmq_consumer/CMakeLists.txt modified: server/modules/filter/CMakeLists.txt --- CMakeLists.txt | 1 - README | 20 ++++++-- macros.cmake | 51 ++++++++++++++++--- .../test/canonical_tests/CMakeLists.txt | 3 ++ rabbitmq_consumer/CMakeLists.txt | 10 +++- server/modules/filter/CMakeLists.txt | 10 ++-- 6 files changed, 72 insertions(+), 23 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 07b4f1af6..38da98860 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,6 @@ cmake_minimum_required(VERSION 2.6) include(macros.cmake) - enable_testing() set_variables() set_maxscale_version() diff --git a/README b/README index 6f8c2dd0a..3affd5b85 100644 --- a/README +++ b/README @@ -166,9 +166,6 @@ values, you can run CMake in interactive mode by using the -i flag or use a CMak It is highly recommended to make a separate build directory to build into. This keeps the source and build trees clean and makes it easy to get rid of everything you built. -By default, MaxScale installs to /usr/local/skysql and places init.d scripts and ldconfig files into their folders. Change the INSTALL_DIR -variable to your desired installation directory and set INSTALL_SYSTEM_FILES=N to prevent the init.d script and ldconfig file installation. - To build MaxScale using CMake: cd @@ -190,20 +187,33 @@ To build MaxScale using the ccmake GUI: ccmake + +If you have your headers and libraries in non-standard locations, you can define those locations at configuration time as such: + + cmake -D= + +By default, MaxScale installs to '/usr/local/skysql/maxscale' and places init.d scripts and ldconfig files into their folders. Change the INSTALL_DIR +variable to your desired installation directory and set INSTALL_SYSTEM_FILES=N to prevent the init.d script and ldconfig file installation. + All the parameters affecting CMake can be found in 'macros.cmake'. This file also has the parameters CMake uses for testing. -Variables controlling the CMake build process: +All the variables that control the CMake build process: INSTALL_DIR= Installation directory BUILD_TYPE=[None|Debug|Release] Type of the build, defaults to Release (optimized) INSTALL_SYSTEM_FILES=[Y|N] Install startup scripts and ld configuration files EMBEDDED_LIB= Path to the embedded library, filename included MYSQL_DIR= Path to MySQL headers +ERRMSG= Path to errmsg.sys file STATIC_EMBEDDED=[Y|N] Link the static or the dynamic verson of the library GCOV=[Y|N] Generate gcov output BUILD_TESTS=[Y|N] Build tests +DEPS_OK=[Y|N] Check dependencies, use N when you want to force a recheck of values DEBUG_OUTPUT=[Y|N] Produce debugging output when configuring CMake - +RABBITMQ_LIB= Path to RabbitMQ-C libraries +RABBITMQ_HEADERS= Path to RabbitMQ-C headers +MYSQL_CLIENT_LIB= Path to MySQL client libraries +MYSQL_CLIENT_HEADERS= Path to MySQL client headers \section Running Running MaxScale diff --git a/macros.cmake b/macros.cmake index 6ffe6dc26..7c5109925 100644 --- a/macros.cmake +++ b/macros.cmake @@ -70,6 +70,7 @@ macro(check_deps) if(DEPS_ERROR) message(FATAL_ERROR "Cannot find dependencies: ${FAILED_DEPS}") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") endif() endmacro() @@ -86,19 +87,23 @@ macro(check_dirs) set(MYSQL_DIR ${MYSQL_DIR_LOC} CACHE PATH "Path to MySQL headers" FORCE) if(${MYSQL_DIR} STREQUAL "MYSQL_DIR-NOTFOUND") message(FATAL_ERROR "Fatal Error: MySQL headers were not found.") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") else() message(STATUS "Using MySQL headers found at: ${MYSQL_DIR}") endif() # Find the errmsg.sys file if it was not defied - if( NOT ( DEFINED ERRMSG ) ) - find_file(ERRMSG errmsg.sys PATHS /usr/share/mysql /usr/local/share/mysql ${CUSTOM_ERRMSG} PATH_SUFFIXES english) - if(${ERRMSG} STREQUAL "ERRMSG-NOTFOUND") - message(FATAL_ERROR "Fatal Error: The errmsg.sys file was not found.") - elseif(DEBUG_OUTPUT) - message(STATUS "Using errmsg.sys found at: ${ERRMSG}") - endif() + if( DEFINED ERRMSG ) + find_file(ERRMSG_FILE errmsg.sys PATHS ${ERRMSG} NO_DEFAULT_PATH) endif() + find_file(ERRMSG_FILE errmsg.sys PATHS /usr/share/mysql /usr/local/share/mysql PATH_SUFFIXES english) + if(${ERRMSG_FILE} MATCHES "ERRMSG_FILE-NOTFOUND") + message(FATAL_ERROR "Fatal Error: The errmsg.sys file was not found, please define the path to it by using -DERRMSG=") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") + else() + message(STATUS "Using errmsg.sys found at: ${ERRMSG_FILE}") + endif() + set(ERRMSG ${ERRMSG_FILE} CACHE FILEPATH "Path to the errmsg.sys file." FORCE) # Find the embedded mysql library if(STATIC_EMBEDDED) @@ -132,6 +137,7 @@ macro(check_dirs) # Inform the user about the embedded library if( (${EMBEDDED_LIB} STREQUAL "EMBEDDED_LIB_STATIC-NOTFOUND") OR (${EMBEDDED_LIB} STREQUAL "EMBEDDED_LIB_DYNAMIC-NOTFOUND")) message(FATAL_ERROR "Library not found: libmysqld. If your install of MySQL is in a non-default location, please provide the location with -DEMBEDDED_LIB=") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") else() get_filename_component(EMBEDDED_LIB ${EMBEDDED_LIB} REALPATH) message(STATUS "Using embedded library: ${EMBEDDED_LIB}") @@ -144,13 +150,42 @@ macro(check_dirs) find_file(DEB_FNC init-functions PATHS /lib/lsb) if(${DEB_FNC} MATCHES "DEB_FNC-NOTFOUND") message(FATAL_ERROR "Cannot find required init-functions in /lib/lsb/ or /etc/rc.d/init.d/, please confirm that your system files are OK.") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") else() set(DEB_BASED TRUE CACHE BOOL "If init.d script uses /lib/lsb/init-functions instead of /etc/rc.d/init.d/functions.") endif() else() set(DEB_BASED FALSE CACHE BOOL "If init.d script uses /lib/lsb/init-functions instead of /etc/rc.d/init.d/functions.") endif() - + + #Check RabbitMQ headers and libraries + if(BUILD_RABBITMQ) + + if(DEFINED RABBITMQ_LIB) + find_library(RMQ_LIB rabbitmq PATHS ${RABBITMQ_LIB} NO_DEFAULT_PATH) + endif() + find_library(RMQ_LIB rabbitmq) + if(RMQ_LIB STREQUAL "RMQ_LIB-NOTFOUND") + message(FATAL_ERROR "Cannot find RabbitMQ libraries, please define the path to the libraries with -DRABBITMQ_LIB=") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") + else() + set(RABBITMQ_LIB ${RMQ_LIB} CACHE PATH "Path to RabbitMQ libraries" FORCE) + message(STATUS "Using RabbitMQ libraries found at: ${RABBITMQ_LIB}") + endif() + + if(DEFINED RABBITMQ_HEADERS) + find_file(RMQ_HEADERS amqp.h PATHS ${RABBITMQ_HEADERS} NO_DEFAULT_PATH) + endif() + find_file(RMQ_HEADERS amqp.h) + if(RMQ_HEADERS STREQUAL "RMQ_HEADERS-NOTFOUND") + message(FATAL_ERROR "Cannot find RabbitMQ headers, please define the path to the headers with -DRABBITMQ_HEADERS=") + set(DEPS_OK FALSE CACHE BOOL "If all the dependencies were found.") + else() + set(RABBITMQ_HEADERS ${RMQ_HEADERS} CACHE PATH "Path to RabbitMQ headers" FORCE) + message(STATUS "Using RabbitMQ headers found at: ${RABBITMQ_HEADERS}") + endif() + + endif() set(DEPS_OK TRUE CACHE BOOL "If all the dependencies were found.") diff --git a/query_classifier/test/canonical_tests/CMakeLists.txt b/query_classifier/test/canonical_tests/CMakeLists.txt index 0575a1a0e..4777fad8d 100644 --- a/query_classifier/test/canonical_tests/CMakeLists.txt +++ b/query_classifier/test/canonical_tests/CMakeLists.txt @@ -1,4 +1,7 @@ file(COPY ${ERRMSG} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) +if(${ERRMSG} MATCHES "ERRMSG-NOTFOUND") + message(FATAL_ERROR "The errmsg.sys file was not found, please define the path with -DERRMSG=") +endif() add_executable(canonizer canonizer.c) target_link_libraries(canonizer pthread query_classifier z dl ssl aio crypt crypto rt m ${EMBEDDED_LIB} fullcore stdc++) add_test(NAME TestCanonicalQuery COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/canontest.sh diff --git a/rabbitmq_consumer/CMakeLists.txt b/rabbitmq_consumer/CMakeLists.txt index 975a4e3e5..aee18a6da 100644 --- a/rabbitmq_consumer/CMakeLists.txt +++ b/rabbitmq_consumer/CMakeLists.txt @@ -1,7 +1,13 @@ -find_library(MYSQL_CLIENT_LIB NAMES mysqlclient PATHS /usr/lib /usr/lib64 PATH_SUFFIXES mysql mariadb) +if (NOT ( DEFINED MYSQL_CLIENT_LIB ) ) + find_library(MYSQL_CLIENT_LIB NAMES mysqlclient PATHS /usr/lib /usr/lib64 PATH_SUFFIXES mysql mariadb) +endif() -if( ( RABBITMQ_LIB AND RABBITMQ_HEADERS ) AND ( NOT ( ${MYSQL_CLIENT_LIB} STREQUAL "MYSQL_CLIENT_LIB-NOTFOUND" ) ) ) +if (NOT ( DEFINED MYSQL_CLIENT_HEADERS ) ) + find_path(MYSQL_CLIENT_HEADERS NAMES mysql.h PATH_SUFFIXES mysql mariadb) +endif() +if( ( RABBITMQ_LIB AND RABBITMQ_HEADERS ) AND ( NOT ( ${MYSQL_CLIENT_LIB} STREQUAL "MYSQL_CLIENT_LIB-NOTFOUND" ) ) AND ( NOT ( ${MYSQL_CLIENT_HEADERS} STREQUAL "MYSQL_CLIENT_HEADERS-NOTFOUND" ) ) ) + include_directories(${MYSQL_CLIENT_HEADERS}) add_executable (consumer consumer.c) target_link_libraries(consumer ${MYSQL_CLIENT_LIB} rabbitmq inih) install(TARGETS consumer DESTINATION bin) diff --git a/server/modules/filter/CMakeLists.txt b/server/modules/filter/CMakeLists.txt index 61c658ace..f9dea236d 100644 --- a/server/modules/filter/CMakeLists.txt +++ b/server/modules/filter/CMakeLists.txt @@ -1,11 +1,7 @@ if(BUILD_RABBITMQ) - if(RABBITMQ_LIB AND RABBITMQ_HEADERS) - add_library(mqfilter SHARED mqfilter.c) - target_link_libraries(mqfilter query_classifier log_manager utils rabbitmq) - install(TARGETS mqfilter DESTINATION modules) - else() - message(ERROR "Error: Cannot find the required librabbitmq-c locations, please check that you have them configured correctly.") - endif() + add_library(mqfilter SHARED mqfilter.c) + target_link_libraries(mqfilter query_classifier log_manager utils rabbitmq) + install(TARGETS mqfilter DESTINATION modules) endif() add_library(regexfilter SHARED regexfilter.c)