From 584d3039baf32d047d0f70df1f1cd40e794287ac Mon Sep 17 00:00:00 2001 From: Hartmut Holzgraefe Date: Tue, 26 Aug 2014 18:20:59 +0200 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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); + } } /*<