From 5c7b2a68e55d57f18bfd3c1fc824f636e7526f51 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Wed, 6 May 2015 12:19:18 +0200 Subject: [PATCH 01/97] mariadb10 compatibility test without GTID First implementation of mariadb10 compatibility test without GTID State machine to be modified for mysql5.6/mariadb10 compatibility router options for mariadb10 slave registration still missing --- server/modules/include/blr.h | 2 +- server/modules/routing/binlog/blr_master.c | 3 ++- server/modules/routing/binlog/blr_slave.c | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 62cda59a3..5c9c70181 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -395,7 +395,7 @@ static char *blrs_states[] = { "Created", "Unregistered", "Registered", #define ANONYMOUS_GTID_EVENT 0x22 #define PREVIOUS_GTIDS_EVENT 0x23 -#define MAX_EVENT_TYPE 0x23 +#define MAX_EVENT_TYPE 0xa3 /** * Binlog event flags diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index 2b164c349..70cd78686 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -448,7 +448,8 @@ char query[128]; GWBUF_CONSUME_ALL(router->saved_master.chksum2); router->saved_master.chksum2 = buf; blr_cache_response(router, "chksum2", buf); - buf = blr_make_query("SELECT @@GLOBAL.GTID_MODE"); + //buf = blr_make_query("SELECT @@GLOBAL.GTID_MODE"); + buf = blr_make_query("SET @mariadb_slave_capability=4); router->master_state = BLRM_GTIDMODE; router->master->func.write(router->master, buf); break; diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 4de69b8d7..8a008e35f 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -369,7 +369,8 @@ int query_len; else if (strcasecmp(word, "@mariadb_slave_capability") == 0) { free(query_text); - return blr_slave_send_ok(router, slave); + return blr_slave_replay(router, slave, router->saved_master.gtid_mode); + //return blr_slave_send_ok(router, slave); } else if (strcasecmp(word, "@master_binlog_checksum") == 0) { From bc7cc2a4666a9ab8946b7134da072bf2703c002d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 7 May 2015 12:56:58 +0300 Subject: [PATCH 02/97] Added variables for MariaDB 10 compatibility. --- server/modules/include/blr.h | 7 +++++-- server/modules/routing/binlog/blr.c | 5 +++++ server/modules/routing/binlog/blr_master.c | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 5c9c70181..48c48a194 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -233,6 +233,7 @@ typedef struct { GWBUF *selectvercom; /*< select @@version_comment */ GWBUF *selecthostname;/*< select @@hostname */ GWBUF *map; /*< select @@max_allowed_packet */ + GWBUF *mariadb10; /*< set @mariadb_slave_capability=4 */ uint8_t *fde_event; /*< Format Description Event */ int fde_len; /*< Length of fde_event */ } MASTER_RESPONSES; @@ -252,6 +253,7 @@ typedef struct router_instance { char *password; /*< Password to use with master */ char *fileroot; /*< Root of binlog filename */ bool master_chksum;/*< Does the master provide checksums */ + bool mariadb10_compat; /*< MariaDB 10.0 compatibility */ char *master_uuid; /*< UUID of the master */ DCB *master; /*< DCB for master connection */ DCB *client; /*< DCB for dummy client */ @@ -314,15 +316,16 @@ typedef struct router_instance { #define BLRM_MAP 0x0011 #define BLRM_REGISTER 0x0012 #define BLRM_BINLOGDUMP 0x0013 - +#define BLRM_MARIADB10 0x0014 #define BLRM_MAXSTATE 0x0013 +#define BLRM_MAXSTATE_MARIADB10 0x0014 static char *blrm_states[] = { "Unconnected", "Connecting", "Authenticated", "Timestamp retrieval", "Server ID retrieval", "HeartBeat Period setup", "binlog checksum config", "binlog checksum rerieval", "GTID Mode retrieval", "Master UUID retrieval", "Set Slave UUID", "Set Names latin1", "Set Names utf8", "select 1", "select version()", "select @@version_comment", "select @@hostname", - "select @@mx_allowed_packet", "Register slave", "Binlog Dump" }; + "select @@mx_allowed_packet", "Register slave", "Binlog Dump","Set MariaDB slave capability" }; #define BLRS_CREATED 0x0000 #define BLRS_UNREGISTERED 0x0001 diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlog/blr.c index 2ba89689f..2f5a38bee 100644 --- a/server/modules/routing/binlog/blr.c +++ b/server/modules/routing/binlog/blr.c @@ -195,6 +195,7 @@ unsigned char *defuuid; inst->retry_backoff = 1; inst->binlogdir = NULL; inst->heartbeat = 300; // Default is every 5 minutes + inst->mariadb10_compat = false; inst->user = strdup(service->credentials.name); inst->password = strdup(service->credentials.authdata); @@ -282,6 +283,10 @@ unsigned char *defuuid; { inst->masterid = atoi(value); } + else if (strcmp(options[i], "mariadb10-compatibility") == 0) + { + inst->mariadb10_compat = config_truth_value(value); + } else if (strcmp(options[i], "filestem") == 0) { inst->fileroot = strdup(value); diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index 70cd78686..bc1e676d0 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -449,7 +449,7 @@ char query[128]; router->saved_master.chksum2 = buf; blr_cache_response(router, "chksum2", buf); //buf = blr_make_query("SELECT @@GLOBAL.GTID_MODE"); - buf = blr_make_query("SET @mariadb_slave_capability=4); + buf = blr_make_query("SET @mariadb_slave_capability=4"); router->master_state = BLRM_GTIDMODE; router->master->func.write(router->master, buf); break; From 8afa46b8b2436dd9bb3680b5642b1c0d3cec7f32 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 7 May 2015 13:00:34 +0300 Subject: [PATCH 03/97] Removed BLRM_MAXSTATE_MARIADB10 and set BLRM_MAXSTATE to 0x014 --- server/modules/include/blr.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 48c48a194..fa377c7e5 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -317,8 +317,7 @@ typedef struct router_instance { #define BLRM_REGISTER 0x0012 #define BLRM_BINLOGDUMP 0x0013 #define BLRM_MARIADB10 0x0014 -#define BLRM_MAXSTATE 0x0013 -#define BLRM_MAXSTATE_MARIADB10 0x0014 +#define BLRM_MAXSTATE 0x0014 static char *blrm_states[] = { "Unconnected", "Connecting", "Authenticated", "Timestamp retrieval", "Server ID retrieval", "HeartBeat Period setup", "binlog checksum config", From e9391ef48639c6607bfcd5472d8996cb74c41614 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 15:16:37 +0200 Subject: [PATCH 04/97] MariaDB 10 optional compatibility MariaDB 10 optional compatibility with mariadb10-compatibility=1 --- server/modules/include/blr.h | 94 +++++++++++----------- server/modules/routing/binlog/blr_master.c | 21 ++++- server/modules/routing/binlog/blr_slave.c | 6 +- 3 files changed, 68 insertions(+), 53 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index fa377c7e5..8aecd83f0 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -233,7 +233,7 @@ typedef struct { GWBUF *selectvercom; /*< select @@version_comment */ GWBUF *selecthostname;/*< select @@hostname */ GWBUF *map; /*< select @@max_allowed_packet */ - GWBUF *mariadb10; /*< set @mariadb_slave_capability=4 */ + GWBUF *mariadb10; /*< set @mariadb_slave_capability */ uint8_t *fde_event; /*< Format Description Event */ int fde_len; /*< Length of fde_event */ } MASTER_RESPONSES; @@ -242,55 +242,54 @@ typedef struct { * The per instance data for the router. */ typedef struct router_instance { - SERVICE *service; /*< Pointer to the service using this router */ - ROUTER_SLAVE *slaves; /*< Link list of all the slave connections */ - SPINLOCK lock; /*< Spinlock for the instance data */ - char *uuid; /*< UUID for the router to use w/master */ - int masterid; /*< Server ID of the master */ - int serverid; /*< Server ID to use with master */ - int initbinlog; /*< Initial binlog file number */ - char *user; /*< User name to use with master */ - char *password; /*< Password to use with master */ - char *fileroot; /*< Root of binlog filename */ - bool master_chksum;/*< Does the master provide checksums */ - bool mariadb10_compat; /*< MariaDB 10.0 compatibility */ - char *master_uuid; /*< UUID of the master */ - DCB *master; /*< DCB for master connection */ - DCB *client; /*< DCB for dummy client */ - SESSION *session; /*< Fake session for master connection */ - unsigned int master_state; /*< State of the master FSM */ - uint8_t lastEventReceived; - uint32_t lastEventTimestamp; /*< Timestamp from last event */ - GWBUF *residual; /*< Any residual binlog event */ - MASTER_RESPONSES saved_master; /*< Saved master responses */ - char *binlogdir; /*< The directory with the binlog files */ - SPINLOCK binlog_lock; /*< Lock to control update of the binlog position */ - char binlog_name[BINLOG_FNAMELEN+1]; + SERVICE *service; /*< Pointer to the service using this router */ + ROUTER_SLAVE *slaves; /*< Link list of all the slave connections */ + SPINLOCK lock; /*< Spinlock for the instance data */ + char *uuid; /*< UUID for the router to use w/master */ + int masterid; /*< Server ID of the master */ + int serverid; /*< Server ID to use with master */ + int initbinlog; /*< Initial binlog file number */ + char *user; /*< User name to use with master */ + char *password; /*< Password to use with master */ + char *fileroot; /*< Root of binlog filename */ + bool master_chksum; /*< Does the master provide checksums */ + bool mariadb10_compat; /*< MariaDB 10.0 compatibility */ + char *master_uuid; /*< UUID of the master */ + DCB *master; /*< DCB for master connection */ + DCB *client; /*< DCB for dummy client */ + SESSION *session; /*< Fake session for master connection */ + unsigned int master_state; /*< State of the master FSM */ + uint8_t lastEventReceived; + uint32_t lastEventTimestamp; /*< Timestamp from last event */ + GWBUF *residual; /*< Any residual binlog event */ + MASTER_RESPONSES saved_master; /*< Saved master responses */ + char *binlogdir; /*< The directory with the binlog files */ + SPINLOCK binlog_lock; /*< Lock to control update of the binlog position */ + char binlog_name[BINLOG_FNAMELEN+1]; /*< Name of the current binlog file */ - uint64_t binlog_position; + uint64_t binlog_position; /*< Current binlog position */ - int binlog_fd; /*< File descriptor of the binlog + int binlog_fd; /*< File descriptor of the binlog * file being written */ - uint64_t last_written; /*< Position of last event written */ - char prevbinlog[BINLOG_FNAMELEN+1]; - int rotating; /*< Rotation in progress flag */ - BLFILE *files; /*< Files used by the slaves */ - SPINLOCK fileslock; /*< Lock for the files queue above */ - unsigned int low_water; /*< Low water mark for client DCB */ - unsigned int high_water; /*< High water mark for client DCB */ - unsigned int short_burst; /*< Short burst for slave catchup */ - unsigned int long_burst; /*< Long burst for slave catchup */ - unsigned long burst_size; /*< Maximum size of burst to send */ - unsigned long heartbeat; /*< Configured heartbeat value */ - ROUTER_STATS stats; /*< Statistics for this router */ - int active_logs; - int reconnect_pending; - int retry_backoff; - time_t connect_time; - int handling_threads; - struct router_instance - *next; + uint64_t last_written; /*< Position of last event written */ + char prevbinlog[BINLOG_FNAMELEN+1]; + int rotating; /*< Rotation in progress flag */ + BLFILE *files; /*< Files used by the slaves */ + SPINLOCK fileslock; /*< Lock for the files queue above */ + unsigned int low_water; /*< Low water mark for client DCB */ + unsigned int high_water; /*< High water mark for client DCB */ + unsigned int short_burst; /*< Short burst for slave catchup */ + unsigned int long_burst; /*< Long burst for slave catchup */ + unsigned long burst_size; /*< Maximum size of burst to send */ + unsigned long heartbeat; /*< Configured heartbeat value */ + ROUTER_STATS stats; /*< Statistics for this router */ + int active_logs; + int reconnect_pending; + int retry_backoff; + time_t connect_time; + int handling_threads; + struct router_instance *next; } ROUTER_INSTANCE; /** @@ -317,6 +316,7 @@ typedef struct router_instance { #define BLRM_REGISTER 0x0012 #define BLRM_BINLOGDUMP 0x0013 #define BLRM_MARIADB10 0x0014 + #define BLRM_MAXSTATE 0x0014 static char *blrm_states[] = { "Unconnected", "Connecting", "Authenticated", "Timestamp retrieval", @@ -324,7 +324,7 @@ static char *blrm_states[] = { "Unconnected", "Connecting", "Authenticated", "Ti "binlog checksum rerieval", "GTID Mode retrieval", "Master UUID retrieval", "Set Slave UUID", "Set Names latin1", "Set Names utf8", "select 1", "select version()", "select @@version_comment", "select @@hostname", - "select @@mx_allowed_packet", "Register slave", "Binlog Dump","Set MariaDB slave capability" }; + "select @@mx_allowed_packet", "Register slave", "Binlog Dump", "Set MariaDB slave capability" }; #define BLRS_CREATED 0x0000 #define BLRS_UNREGISTERED 0x0001 diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index bc1e676d0..2efa417e4 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -448,12 +448,27 @@ char query[128]; GWBUF_CONSUME_ALL(router->saved_master.chksum2); router->saved_master.chksum2 = buf; blr_cache_response(router, "chksum2", buf); - //buf = blr_make_query("SELECT @@GLOBAL.GTID_MODE"); - buf = blr_make_query("SET @mariadb_slave_capability=4"); - router->master_state = BLRM_GTIDMODE; + + if (router->mariadb10_compat) { + buf = blr_make_query("SET @mariadb_slave_capability=4"); + router->master_state = BLRM_MARIADB10; + } else { + buf = blr_make_query("SELECT @@GLOBAL.GTID_MODE"); + router->master_state = BLRM_GTIDMODE; + } router->master->func.write(router->master, buf); break; } + case BLRM_MARIADB10: + // Response to the SET @mariadb_slave_capability=4, should be stored + if (router->saved_master.mariadb10) + GWBUF_CONSUME_ALL(router->saved_master.mariadb10); + router->saved_master.mariadb10 = buf; + blr_cache_response(router, "mariadb10", buf); + buf = blr_make_registration(router); + router->master_state = BLRM_REGISTER; + router->master->func.write(router->master, buf); + break; case BLRM_GTIDMODE: // Response to the GTID_MODE, should be stored if (router->saved_master.gtid_mode) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 8a008e35f..fe2bc2dff 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -366,11 +366,11 @@ int query_len; free(query_text); return blr_slave_replay(router, slave, router->saved_master.heartbeat); } - else if (strcasecmp(word, "@mariadb_slave_capability") == 0) + else if (strcasecmp(word, "@mariadb_slave_capability") == 0) { free(query_text); - return blr_slave_replay(router, slave, router->saved_master.gtid_mode); - //return blr_slave_send_ok(router, slave); + if (router->mariadb10_compat) + return blr_slave_replay(router, slave, router->saved_master.mariadb10); } else if (strcasecmp(word, "@master_binlog_checksum") == 0) { From 3f2876bde35d11464638c88cc3c0f52ff13316ee Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 15:32:12 +0200 Subject: [PATCH 05/97] Fixed buffer free Fixed buffer free --- server/modules/routing/binlog/blr_slave.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index fe2bc2dff..1928e5d9d 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -368,9 +368,10 @@ int query_len; } else if (strcasecmp(word, "@mariadb_slave_capability") == 0) { - free(query_text); - if (router->mariadb10_compat) + if (router->mariadb10_compat) { + free(query_text); return blr_slave_replay(router, slave, router->saved_master.mariadb10); + } } else if (strcasecmp(word, "@master_binlog_checksum") == 0) { From 2c2a03a6f696894cb015f32a0ae9d2ca50123806 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 16:10:35 +0200 Subject: [PATCH 06/97] Always reply to SET @mariadb_slave_capability Always reply to SET @mariadb_slave_capability, with saved master reply for mariadb10 master or with OK otherwise --- server/modules/routing/binlog/blr_slave.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 1928e5d9d..c963d8ab4 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -368,9 +368,11 @@ int query_len; } else if (strcasecmp(word, "@mariadb_slave_capability") == 0) { + free(query_text); if (router->mariadb10_compat) { - free(query_text); return blr_slave_replay(router, slave, router->saved_master.mariadb10); + } else { + return blr_slave_send_ok(router, slave); } } else if (strcasecmp(word, "@master_binlog_checksum") == 0) From 7d48779913c7ebf62bd1eed7ce5a3f437bfee0ef Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 17:02:33 +0200 Subject: [PATCH 07/97] Added MAX_EVENT_TYPE_MARIADB10 check Added MAX_EVENT_TYPE_MARIADB10 check for router->mariadb10_compat --- server/modules/include/blr.h | 3 ++- server/modules/routing/binlog/blr_file.c | 23 +++++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 8aecd83f0..7b57fd15f 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -397,7 +397,8 @@ static char *blrs_states[] = { "Created", "Unregistered", "Registered", #define ANONYMOUS_GTID_EVENT 0x22 #define PREVIOUS_GTIDS_EVENT 0x23 -#define MAX_EVENT_TYPE 0xa3 +#define MAX_EVENT_TYPE 0x23 +#define MAX_EVENT_TYPE_MARIADB10 0xa3 /** * Binlog event flags diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlog/blr_file.c index 2473909ec..ec7864d5e 100644 --- a/server/modules/routing/binlog/blr_file.c +++ b/server/modules/routing/binlog/blr_file.c @@ -443,15 +443,26 @@ struct stat statb; hdr->next_pos = EXTRACT32(&hdbuf[13]); hdr->flags = EXTRACT16(&hdbuf[17]); - if (hdr->event_type > MAX_EVENT_TYPE) - { - LOGIF(LE, (skygw_log_write(LOGFILE_ERROR, - "Invalid event type 0x%x. " + if (router->mariadb10_compat) { + if (hdr->event_type > MAX_EVENT_TYPE_MARIADB10) { + LOGIF(LE, (skygw_log_write(LOGFILE_ERROR, + "Invalid MariaDB 10 event type 0x%x. " "Binlog file is %s, position %d", hdr->event_type, file->binlogname, pos))); - return NULL; - } + return NULL; + } + } else { + if (hdr->event_type > MAX_EVENT_TYPE) { + LOGIF(LE, (skygw_log_write(LOGFILE_ERROR, + "Invalid event type 0x%x. " + "Binlog file is %s, position %d", + hdr->event_type, + file->binlogname, pos))); + + return NULL; + } + } if (hdr->next_pos < pos && hdr->event_type != ROTATE_EVENT) { From 5d1e09ca4f22ba0a5402a089173cf4652ad71e08 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 17:14:39 +0200 Subject: [PATCH 08/97] Added MariaDB 10 Compatibility without GTID Added MariaDB 10 Compatibility without GTID --- server/modules/routing/binlog/blr.c | 2 ++ server/modules/routing/binlog/blr_file.c | 3 ++- server/modules/routing/binlog/blr_master.c | 3 ++- server/modules/routing/binlog/blr_slave.c | 2 ++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlog/blr.c index 2f5a38bee..071563952 100644 --- a/server/modules/routing/binlog/blr.c +++ b/server/modules/routing/binlog/blr.c @@ -35,6 +35,8 @@ * 02/04/2014 Mark Riddoch Initial implementation * 17/02/2015 Massimiliano Pinto Addition of slave port and username in diagnostics * 18/02/2015 Massimiliano Pinto Addition of dcb_close in closeSession + * 07/05/2015 Massimiliano Pinto Addition of MariaDB 10 compatibility support + * * @endverbatim */ diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlog/blr_file.c index ec7864d5e..e59a7be19 100644 --- a/server/modules/routing/binlog/blr_file.c +++ b/server/modules/routing/binlog/blr_file.c @@ -23,8 +23,9 @@ * @verbatim * Revision History * - * Date Who Description + * Date Who Description * 14/04/2014 Mark Riddoch Initial implementation + * 07/05/2015 Massimiliano Pinto Added MAX_EVENT_TYPE_MARIADB10 * * @endverbatim */ diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index 2efa417e4..f230442f1 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -31,8 +31,9 @@ * @verbatim * Revision History * - * Date Who Description + * Date Who Description * 02/04/2014 Mark Riddoch Initial implementation + * 07/05/2015 Massimiliano Pinto Added MariaDB 10 Compatibility * * @endverbatim */ diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index c963d8ab4..141ff3a03 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -36,9 +36,11 @@ * 18/02/2015 Massimiliano Pinto Addition of DISCONNECT ALL and DISCONNECT SERVER server_id * 18/03/2015 Markus Makela Better detection of CRC32 | NONE checksum * 19/03/2015 Massimiliano Pinto Addition of basic MariaDB 10 compatibility support + * 07/05/2015 Massimiliano Pinto Added MariaDB 10 Compatibility * * @endverbatim */ + #include #include #include From 230f88737cca9e6a5811f7261a2bc63eff0adf6a Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 7 May 2015 18:05:04 +0200 Subject: [PATCH 09/97] Added reading saved mariadb10 data Added reading saved mariadb10 data --- server/modules/routing/binlog/blr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlog/blr.c index 071563952..becea18b0 100644 --- a/server/modules/routing/binlog/blr.c +++ b/server/modules/routing/binlog/blr.c @@ -395,6 +395,7 @@ unsigned char *defuuid; inst->saved_master.selectvercom = blr_cache_read_response(inst, "selectvercom"); inst->saved_master.selecthostname = blr_cache_read_response(inst, "selecthostname"); inst->saved_master.map = blr_cache_read_response(inst, "map"); + inst->saved_master.mariadb10 = blr_cache_read_response(inst, "mariadb10"); /* * Initialise the binlog file and position From f991e58b5714b82952b19d4283b0117f414e01d6 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 11 May 2015 11:43:21 +0200 Subject: [PATCH 10/97] MariaDB 10 master requires MariaDB 10 slaves Only MariaDB 10 slaves can register to binblog server with a MariaDB 10 Master --- server/modules/include/blr.h | 6 ++++-- server/modules/routing/binlog/blr.c | 1 + server/modules/routing/binlog/blr_slave.c | 26 ++++++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 7b57fd15f..3dbc43853 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -24,8 +24,9 @@ * @verbatim * Revision History * - * Date Who Description - * 02/04/14 Mark Riddoch Initial implementation + * Date Who Description + * 02/04/14 Mark Riddoch Initial implementation + * 11/05/15 Massimilaino Pinto Added mariadb10_compat to master and slave structs * * @endverbatim */ @@ -175,6 +176,7 @@ typedef struct router_slave { uint32_t lastEventTimestamp;/*< Last event timestamp sent */ SPINLOCK catch_lock; /*< Event catchup lock */ unsigned int cstate; /*< Catch up state */ + bool mariadb10_compat;/*< MariaDB 10.0 compatibility */ SPINLOCK rses_lock; /*< Protects rses_deleted */ pthread_t pthread; struct router_instance diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlog/blr.c index becea18b0..8dce5ea58 100644 --- a/server/modules/routing/binlog/blr.c +++ b/server/modules/routing/binlog/blr.c @@ -498,6 +498,7 @@ ROUTER_SLAVE *slave; strcpy(slave->binlogfile, "unassigned"); slave->connect_time = time(0); slave->lastEventTimestamp = 0; + slave->mariadb10_compat = false; /** * Add this session to the list of active sessions. diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 141ff3a03..b932000d3 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -37,6 +37,7 @@ * 18/03/2015 Markus Makela Better detection of CRC32 | NONE checksum * 19/03/2015 Massimiliano Pinto Addition of basic MariaDB 10 compatibility support * 07/05/2015 Massimiliano Pinto Added MariaDB 10 Compatibility + * 11/05/2015 Massimiliano Pinto Only MariaDB 10 Slaves can register to binlog router with a MariaDB 10 Master * * @endverbatim */ @@ -125,7 +126,27 @@ blr_slave_request(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) return blr_slave_query(router, slave, queue); break; case COM_REGISTER_SLAVE: - return blr_slave_register(router, slave, queue); + /* + * If Master is MariaDB10 don't allow registration from + * MariaDB/Mysql 5 Slaves + */ + + if (router->mariadb10_compat && !slave->mariadb10_compat) { + slave->state = BLRS_ERRORED; + blr_send_custom_error(slave->dcb, 1, 0, + "MariaDB 10 Slave is required for Slave registration"); + + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "MariaDB 10 Slave is required for Slave registration", + MYSQL_COMMAND(queue)))); + + dcb_close(slave->dcb); + return 1; + } else { + /* Master and Slave version OK: continue with slave registration */ + return blr_slave_register(router, slave, queue); + } break; case COM_BINLOG_DUMP: return blr_slave_binlog_dump(router, slave, queue); @@ -370,6 +391,9 @@ int query_len; } else if (strcasecmp(word, "@mariadb_slave_capability") == 0) { + /* mariadb10 compatibility is set for the slave */ + slave->mariadb10_compat=true; + free(query_text); if (router->mariadb10_compat) { return blr_slave_replay(router, slave, router->saved_master.mariadb10); From a48e694dba044ae25debf846585c4079f17a679b Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 11 May 2015 12:42:14 +0200 Subject: [PATCH 11/97] Fix for log messages Fix for log messages about MariaDB 10 registration and unexpected query --- server/modules/routing/binlog/blr_slave.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index b932000d3..87b795ea6 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -138,8 +138,9 @@ blr_slave_request(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) LOGIF(LE, (skygw_log_write( LOGFILE_ERROR, - "MariaDB 10 Slave is required for Slave registration", - MYSQL_COMMAND(queue)))); + "%s: Slave %s: a MariaDB 10 Slave is required for Slave registration", + router->service->name, + slave->dcb->remote))); dcb_close(slave->dcb); return 1; @@ -472,7 +473,7 @@ int query_len; query_text = strndup(qtext, query_len); LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, "Unexpected query from slave server %s", query_text))); + LOGFILE_ERROR, "Unexpected query from slave %s: %s", slave->dcb->remote, query_text))); free(query_text); blr_slave_send_error(router, slave, "Unexpected SQL query received from slave."); return 1; From 2f2c9c8cbc7d4f6fc343f99c8c5e9f1bd0f7b279 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Tue, 2 Jun 2015 09:45:26 +0200 Subject: [PATCH 12/97] Fix for MariaDB10 state machine Fix for MariaDB10 state machine --- server/modules/routing/binlog/blr_master.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index f230442f1..3a0519108 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -466,8 +466,8 @@ char query[128]; GWBUF_CONSUME_ALL(router->saved_master.mariadb10); router->saved_master.mariadb10 = buf; blr_cache_response(router, "mariadb10", buf); - buf = blr_make_registration(router); - router->master_state = BLRM_REGISTER; + buf = blr_make_query("SHOW VARIABLES LIKE 'SERVER_UUID'"); + router->master_state = BLRM_MUUID; router->master->func.write(router->master, buf); break; case BLRM_GTIDMODE: From 94ba445d2fad80fd9dc7075db792ced8a3ea2001 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 28 May 2015 10:30:21 +0300 Subject: [PATCH 13/97] Added 5.5.5- string to the start of MariaDB 10.0 version strings. --- server/core/config.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/server/core/config.c b/server/core/config.c index d22bb777e..a2414ea7a 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -231,7 +231,7 @@ int rval; strcpy(version_string,tmp); } - ptr = strstr(tmp, "-embedded"); + ptr = strstr(version_string, "-embedded"); if (ptr) { *ptr = '\0'; } @@ -417,7 +417,21 @@ hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); } if (version_string) { + + /** Add the 5.5.5- string to the start of the version string if + * the version string starts with "10.". + * This mimics MariaDB 10.0 replication which adds 5.5.5- for backwards compatibility. */ + if(strncmp(version_string,"10.",3) == 0) + { + ((SERVICE *)(obj->element))->version_string = malloc((strlen(version_string) + + strlen("5.5.5-") + 1) * sizeof(char)); + strcpy(((SERVICE *)(obj->element))->version_string,"5.5.5-"); + strcat(((SERVICE *)(obj->element))->version_string,version_string); + } + else + { ((SERVICE *)(obj->element))->version_string = strdup(version_string); + } } else { if (gateway.version_string) ((SERVICE *)(obj->element))->version_string = strdup(gateway.version_string); From 1275a594acf71d276f6c3390cceb9ec58cd4f151 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 7 May 2015 05:56:28 +0300 Subject: [PATCH 14/97] Added missing utils library link from testmodutils. --- server/core/test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/test/CMakeLists.txt b/server/core/test/CMakeLists.txt index 82e58919f..5c1f9a8ad 100644 --- a/server/core/test/CMakeLists.txt +++ b/server/core/test/CMakeLists.txt @@ -21,7 +21,7 @@ target_link_libraries(test_spinlock fullcore log_manager) target_link_libraries(test_filter fullcore) target_link_libraries(test_buffer fullcore log_manager) target_link_libraries(test_dcb fullcore) -target_link_libraries(test_modutil fullcore) +target_link_libraries(test_modutil fullcore utils log_manager) target_link_libraries(test_poll fullcore) target_link_libraries(test_service fullcore) target_link_libraries(test_server fullcore) From e14b29baf90f7ab3f62ea5a0314b0b1da0e5a7d5 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 21 Jun 2015 12:51:54 +0300 Subject: [PATCH 15/97] Fix to MXS-212: https://mariadb.atlassian.net/browse/MXS-212 The listener DCB is now properly closed instead of just being removed from the poll set. --- server/core/dcb.c | 8 +++++++- server/core/poll.c | 3 ++- server/core/service.c | 19 +++++++++---------- server/modules/protocol/mysql_client.c | 2 +- server/modules/protocol/mysql_common.c | 5 ++++- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7f3651953..fc9329524 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1949,13 +1949,15 @@ dcb_close(DCB *dcb) } ss_dassert(dcb->state == DCB_STATE_POLLING || + dcb->state == DCB_STATE_LISTENING || dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE); /*< * Stop dcb's listening and modify state accordingly. */ - if (dcb->state == DCB_STATE_POLLING) + if (dcb->state == DCB_STATE_POLLING || + dcb->state == DCB_STATE_LISTENING) { rc = poll_remove_dcb(dcb); @@ -2428,6 +2430,10 @@ static bool dcb_set_state_nomutex( case DCB_STATE_POLLING: /*< ok to try but state can't change */ succp = true; break; + case DCB_STATE_LISTENING: + dcb->state = new_state; + succp = true; + break; default: ss_dassert(old_state != NULL); break; diff --git a/server/core/poll.c b/server/core/poll.c index 377310cb0..9a1a5565d 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -330,7 +330,8 @@ poll_remove_dcb(DCB *dcb) CHK_DCB(dcb); /*< It is possible that dcb has already been removed from the set */ - if (dcb->state != DCB_STATE_POLLING) + if (dcb->state != DCB_STATE_POLLING && + dcb->state != DCB_STATE_LISTENING) { if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) diff --git a/server/core/service.c b/server/core/service.c index b0959e6c3..c11c0bddc 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,11 +534,13 @@ int listeners = 0; port = service->ports; while (port) { - poll_remove_dcb(port->listener); - port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; + if(port->listener) + { + dcb_close(port->listener); + port->listener = NULL; listeners++; - - port = port->next; + } + port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -562,13 +564,10 @@ int listeners = 0; port = service->ports; while (port) { - if (poll_add_dcb(port->listener) == 0) { - port->listener->session->state = SESSION_STATE_LISTENER; - listeners++; - } - port = port->next; + listeners += serviceStartPort(service,port); + port = port->next; } - + service->state = SERVICE_STATE_STARTED; return listeners; } diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index c3e463139..21a11dc42 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1771,7 +1771,7 @@ gw_client_close(DCB *dcb) dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) { - if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(protocol); + if (!DCB_IS_CLONE(dcb) && protocol) CHK_PROTOCOL(protocol); } #endif LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 83e2912e5..eb044c1fb 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -128,7 +128,10 @@ void mysql_protocol_done ( MySQLProtocol* p; server_command_t* scmd; server_command_t* scmd2; - + + if(dcb->protocol == NULL) + return; + p = (MySQLProtocol *)dcb->protocol; spinlock_acquire(&p->protocol_lock); From c3aa5beeb45abc921bdc49a751e20269bea394ac Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 21 Jun 2015 19:32:19 +0300 Subject: [PATCH 16/97] Added missing initialization from MM monitor. --- server/modules/monitor/mmmon.c | 1 + 1 file changed, 1 insertion(+) diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index c0d54595d..545a77ca2 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -126,6 +126,7 @@ startMonitor(void *arg,void* opt) handle->shutdown = 0; handle->id = MONITOR_DEFAULT_ID; handle->master = NULL; + handle->script = NULL; memset(handle->events,false,sizeof(handle->events)); spinlock_init(&handle->lock); } From 8c900e73deab80691e004ee3319981adf173dd4e Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 22 Jun 2015 10:46:00 +0200 Subject: [PATCH 17/97] removed extra blr_file_add_magic removed extra blr_file_add_magic --- server/modules/include/blr.h | 1 + server/modules/routing/binlog/blr_file.c | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/server/modules/include/blr.h b/server/modules/include/blr.h index 62cda59a3..72171b253 100644 --- a/server/modules/include/blr.h +++ b/server/modules/include/blr.h @@ -34,6 +34,7 @@ #include #include +#include #define BINLOG_FNAMELEN 16 #define BLR_PROTOCOL "MySQLBackend" diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlog/blr_file.c index ca92d6d06..68f47b1c3 100644 --- a/server/modules/routing/binlog/blr_file.c +++ b/server/modules/routing/binlog/blr_file.c @@ -210,9 +210,8 @@ int fd; close(router->binlog_fd); spinlock_acquire(&router->binlog_lock); strncpy(router->binlog_name, file,BINLOG_FNAMELEN); - blr_file_add_magic(router, fd); - spinlock_release(&router->binlog_lock); router->binlog_fd = fd; + spinlock_release(&router->binlog_lock); return 1; } @@ -254,12 +253,13 @@ int fd; LOGIF(LE, (skygw_log_write(LOGFILE_ERROR, "%s: binlog file %s has an invalid length %d.", router->service->name, path, router->binlog_position))); - close(fd); + close(fd); + spinlock_release(&router->binlog_lock); return; } } - spinlock_release(&router->binlog_lock); router->binlog_fd = fd; + spinlock_release(&router->binlog_lock); } /** From 15e6d6f9fcb53eb0d95bb3a4e6e082815e72f2ec Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 22 Jun 2015 18:24:39 +0200 Subject: [PATCH 18/97] fix for missing crc check in blr_slave_fake_rotate() fix for missing crc check in blr_slave_fake_rotate() --- server/modules/routing/binlog/blr_slave.c | 27 ++++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 4de69b8d7..ee7d52099 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -1693,6 +1693,9 @@ uint32_t chksum; binlognamelen = strlen(slave->binlogfile); len = 19 + 8 + 4 + binlognamelen; + /* no slave crc, remove 4 bytes */ + if (slave->nocrc) + len -= 4; // Build a fake rotate event resp = gwbuf_alloc(len + 5); @@ -1711,17 +1714,19 @@ uint32_t chksum; memcpy(ptr, slave->binlogfile, binlognamelen); ptr += binlognamelen; - /* - * Now add the CRC to the fake binlog rotate event. - * - * The algorithm is first to compute the checksum of an empty buffer - * and then the checksum of the event portion of the message, ie we do not - * include the length, sequence number and ok byte that makes up the first - * 5 bytes of the message. We also do not include the 4 byte checksum itself. - */ - chksum = crc32(0L, NULL, 0); - chksum = crc32(chksum, GWBUF_DATA(resp) + 5, hdr.event_size - 4); - encode_value(ptr, chksum, 32); + if (!slave->nocrc) { + /* + * Now add the CRC to the fake binlog rotate event. + * + * The algorithm is first to compute the checksum of an empty buffer + * and then the checksum of the event portion of the message, ie we do not + * include the length, sequence number and ok byte that makes up the first + * 5 bytes of the message. We also do not include the 4 byte checksum itself. + */ + chksum = crc32(0L, NULL, 0); + chksum = crc32(chksum, GWBUF_DATA(resp) + 5, hdr.event_size - 4); + encode_value(ptr, chksum, 32); + } slave->dcb->func.write(slave->dcb, resp); return 1; From c22c6ea46aaaad1cfba993ccd19a41cbea4abe6e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 09:15:06 +0300 Subject: [PATCH 19/97] ServiceStop only removed DCBs from the polling system This removes the need to establish new DCBs for each of the listeners while still blocking new session creation for a service which is shut down. The client will not receive an error and the connection will be accepted when the service is restarted. --- server/core/service.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index c11c0bddc..cfce442b8 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,12 +534,8 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener) - { - dcb_close(port->listener); - port->listener = NULL; + if(poll_remove_dcb(port->listener) == 0) listeners++; - } port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -564,7 +560,8 @@ int listeners = 0; port = service->ports; while (port) { - listeners += serviceStartPort(service,port); + if(poll_add_dcb(port->listener) == 0) + listeners++; port = port->next; } service->state = SERVICE_STATE_STARTED; From dc43a7d9da855015b1013980d1227c76c845e015 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:25:59 +0300 Subject: [PATCH 20/97] Removed unnecessary code from dcb_close and dcb_set_state_nomutex. --- server/core/dcb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index fc9329524..7f3651953 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1949,15 +1949,13 @@ dcb_close(DCB *dcb) } ss_dassert(dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_LISTENING || dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE); /*< * Stop dcb's listening and modify state accordingly. */ - if (dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_LISTENING) + if (dcb->state == DCB_STATE_POLLING) { rc = poll_remove_dcb(dcb); @@ -2430,10 +2428,6 @@ static bool dcb_set_state_nomutex( case DCB_STATE_POLLING: /*< ok to try but state can't change */ succp = true; break; - case DCB_STATE_LISTENING: - dcb->state = new_state; - succp = true; - break; default: ss_dassert(old_state != NULL); break; From 3de7798fac7ffc74d36d50316e1449b3ab333bb2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:49:27 +0300 Subject: [PATCH 21/97] Added missing session state changes. --- server/core/service.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/core/service.c b/server/core/service.c index cfce442b8..bce7328cc 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -535,7 +535,10 @@ int listeners = 0; while (port) { if(poll_remove_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; listeners++; + } port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -561,7 +564,10 @@ int listeners = 0; while (port) { if(poll_add_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER; listeners++; + } port = port->next; } service->state = SERVICE_STATE_STARTED; From 3d7b050f6ff3df7cc939903a8db202e938168be3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:53:29 +0300 Subject: [PATCH 22/97] Removed unnecessary NULL checks in mysql_client and mysql_common --- server/modules/protocol/mysql_client.c | 2 +- server/modules/protocol/mysql_common.c | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 21a11dc42..c3e463139 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1771,7 +1771,7 @@ gw_client_close(DCB *dcb) dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) { - if (!DCB_IS_CLONE(dcb) && protocol) CHK_PROTOCOL(protocol); + if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(protocol); } #endif LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index eb044c1fb..83e2912e5 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -128,10 +128,7 @@ void mysql_protocol_done ( MySQLProtocol* p; server_command_t* scmd; server_command_t* scmd2; - - if(dcb->protocol == NULL) - return; - + p = (MySQLProtocol *)dcb->protocol; spinlock_acquire(&p->protocol_lock); From abf39303d7590e908a54de7d3309183ab5efa5d7 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 16:20:26 +0300 Subject: [PATCH 23/97] Fixed the wrong value being returned form dcb_read_SSL. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7f3651953..9e94150a8 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1239,7 +1239,7 @@ int dcb_read_SSL( } /*< while (true) */ return_n: - return n; + return nread; } /** * General purpose routine to write to a DCB From 039cbff1811315606bdec20dee421d9521830fe1 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 16:45:10 +0300 Subject: [PATCH 24/97] Added missing null checks. --- server/core/service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index bce7328cc..7966d33ec 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,7 +534,7 @@ int listeners = 0; port = service->ports; while (port) { - if(poll_remove_dcb(port->listener) == 0) + if(port->listener && poll_remove_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; listeners++; @@ -563,7 +563,7 @@ int listeners = 0; port = service->ports; while (port) { - if(poll_add_dcb(port->listener) == 0) + if(port->listener && poll_add_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER; listeners++; From c42d3d9f7ac086b5e012d9c2ee64f3b024c9ed17 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 17:05:31 +0300 Subject: [PATCH 25/97] Added missing NULL checks. --- server/core/dcb.c | 3 +++ server/core/service.c | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7f3651953..07bb1dc93 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2423,6 +2423,9 @@ static bool dcb_set_state_nomutex( case DCB_STATE_NOPOLLING: switch (new_state) { + /** Stopped services which are restarting will go from + * DCB_STATE_NOPOLLING to DCB_STATE_LISTENING.*/ + case DCB_STATE_LISTENING: case DCB_STATE_ZOMBIE: /*< fall through */ dcb->state = new_state; case DCB_STATE_POLLING: /*< ok to try but state can't change */ diff --git a/server/core/service.c b/server/core/service.c index 7966d33ec..24d51542d 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,10 +534,14 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener && poll_remove_dcb(port->listener) == 0) + if(port->listener && + port->listener->session->state == SESSION_STATE_LISTENER) { - port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; - listeners++; + if(poll_remove_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; + listeners++; + } } port = port->next; } @@ -563,11 +567,15 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener && poll_add_dcb(port->listener) == 0) + if(port->listener && + port->listener->session->state == SESSION_STATE_LISTENER_STOPPED) + { + if(poll_add_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER; listeners++; } + } port = port->next; } service->state = SERVICE_STATE_STARTED; From 77c085fbd3be191f2b4679a628d89fb272814b02 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 19:51:19 +0300 Subject: [PATCH 26/97] Improved routing hint documentation. --- Documentation/Reference/Hint-Syntax.md | 83 +++++++++++++++++++++----- 1 file changed, 69 insertions(+), 14 deletions(-) diff --git a/Documentation/Reference/Hint-Syntax.md b/Documentation/Reference/Hint-Syntax.md index 264d264f6..b84bb85fd 100644 --- a/Documentation/Reference/Hint-Syntax.md +++ b/Documentation/Reference/Hint-Syntax.md @@ -1,42 +1,97 @@ # Hint Syntax -Use either ’-- ’ (notice the whitespace) or ’#’ after the semicolon or ’/* .. */’ before -the semicolon. +## Enabling routing hints -The MySQL manual doesn’t specify if comment blocks, i.e. ’/* .. */’, should contain a w -hitespace character before or after the tags. +To enable routing hints for a service, the hintfilter module needs to be configured and the filter needs to be applied to the service. -All hints must start with the ’maxscale tag’: - -- maxscale - -The hints right now have two types, ones that route to a server and others that contain +Here is an example service which has the hint filter configured and applied. + +``` +[Read Service] +type=service +router=readconnroute +router_options=master +servers=server1 +user=maxuser +passwd=maxpwd +filter=Hint + +[Hint] +type=filter +module=hintfilter + +``` + +## Comments and comment types + +The client connection will need to have comments enabled. For example the `mysql` command line client has comments disabled by default. + +For comment types, use either `-- ` (notice the whitespace) or `#` after the semicolon or `/* .. */` before the semicolon. All comment types work with routing hints. + +The MySQL manual doesn`t specify if comment blocks, i.e. `/* .. */`, should contain a w +hitespace character before or after the tags, so adding whitespace at both the start and the end is advised. + +## Hint body + +All hints must start with the `maxscale` tag. + +``` +-- maxscale +``` + +The hints have two types, ones that route to a server and others that contain name-value pairs. -Routing queries to a server: +###Routing destination hints + +These hints will instruct the router to route a query to a certain type of a server. +``` -- maxscale route to [master | slave | server ] +``` -The name of the server is the same as in maxscale.cnf +A `master` value in a routing hint will route the query to a master server. This can be used to direct read queries to a master server for a up-to-date result with no replication lag. A `slave` value will route the query to a slave server. A `server` value will route the query to a named server. The value of needs to be the same as the server section name in maxscale.cnf. -Creating a name-value pair: +### Name-value hints + +These control the behavior and affect the routing decisions made by the router. + +``` -- maxscale = +``` -Currently the only accepted parameter is -’max_slave_replication_lag’ +Currently the only accepted parameter is `max_slave_replication_lag`. This will route the query to a server with lower replication lag then what is defined in the hint value. + +## Hint stack Hints can be either single-use hints, which makes them affect only one query, or named hints, which can be pushed on and off a stack of active hints. Defining named hints: + +``` -- maxscale prepare +``` Pushing a hint onto the stack: + +``` -- maxscale begin +``` Popping the topmost hint off the stack: + +``` -- maxscale end +``` You can define and activate a hint in a single command using the following: + +``` -- maxscale begin +``` You can also push anonymous hints onto the stack which are only used as long as they are on the stack: --- maxscale begin \ No newline at end of file + +``` +-- maxscale begin +``` From 29492426c3e48b30cb0273d0b20ea9315035751b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 20:06:02 +0300 Subject: [PATCH 27/97] Updated documentation contents. --- Documentation/Documentation-Contents.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index 99f8ba38e..fc51d83ec 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -5,7 +5,7 @@ ## About MaxScale - [About MaxScale](About/About-MaxScale.md) - - [Release Notes 1.1](Release-Notes/MaxScale-1.1-Release-Notes.md) + - [MaxScale 1.2.0 Release Notes](Release-Notes/MaxScale-1.2.0-Release-Notes.md) - [Changelog](Changelog.md) - [Limitations](About/Limitations.md) - [COPYRIGHT](About/COPYRIGHT.md) @@ -28,6 +28,7 @@ - [MaxScale HA with Lsyncd](Reference/MaxScale-HA-with-lsyncd.md) - [How Errors are Handled in MaxScale](Reference/How-errors-are-handled-in-MaxScale.md) - [Debug and Diagnostic Support](Reference/Debug-And-Diagnostic-Support.md) + - [Routing Hints](Reference/Hint-Syntax.md) ## Tutorials @@ -70,10 +71,6 @@ Here are detailed documents about the filters MaxScale offers. They contain conf - [RabbitMQ Consumer Client](filters/RabbitMQ-Consumer-Client.md) -## Routers - - - [Simple Schema Sharding Router](routers/schemarouter/SchemaRouter.md) - ## Design Documents - [Core Objects Design (in development)](http://mariadb-corporation.github.io/MaxScale/Design-Documents/core-objects-html-docs) @@ -89,4 +86,7 @@ Here are detailed documents about the filters MaxScale offers. They contain conf - [MaxScale 1.0 Release Notes](Release-Notes/MaxScale-1.0-Release-Notes.md) - [MaxScale 1.0.1 Release Notes](Release-Notes/MaxScale-1.0.1-Release-Notes.md) - [MaxScale 1.0.3 Release Notes](Release-Notes/MaxScale-1.0.3-Release-Notes.md) + - [MaxScale 1.1.0 Release Notes](Release-Notes/MaxScale-1.1-Release-Notes.md) + - [MaxScale 1.1.1 Release Notes](Release-Notes/MaxScale-1.1.1-Release-Notes.md) + - [MaxScale 1.2.0 Release Notes](Release-Notes/MaxScale-1.2.0-Release-Notes.md) From f44a3cf758ded11f85bb6fd2ab803e76061e2ac7 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 20:10:43 +0300 Subject: [PATCH 28/97] Updated 1.2 release notes. --- Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index f85382b3c..a84f94c27 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -24,3 +24,8 @@ A quick list of changes in installation directories and file names: * Data directory is `/var/lib/maxscale`. This is the default location for MaxScale-specific data. * PID file can be found at `/var/run/maxscale` +### Client side SSL encryption +MaxScale now supports SSL/TLS encrypted connections to MaxScale. + +### Monitor scripts +State changes in backend servers can now trigger the execution of a custom script. With this you can easily customize MaxScale's behavior. From 13fb88ea076c80da79182ec2ee5d9123ae6cd075 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 11:29:43 +0300 Subject: [PATCH 29/97] Added optional code for older OpenSSL library versions. --- CMakeLists.txt | 14 +++++++++++++- server/core/gateway.c | 14 +++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3b040f0df..c28b2c657 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,19 +31,31 @@ include(cmake/CheckPlatform.cmake) check_deps() check_dirs() +find_package(OpenSSL) find_package(Valgrind) find_package(MySQLClient) find_package(MySQL) find_package(Pandoc) find_package(TCMalloc) find_package(Jemalloc) +find_package(CURL) # You can find the variables set by this in the FindCURL.cmake file # which is a default module in CMake. -find_package(CURL) + if(NOT CURL_FOUND) message(FATAL_ERROR "Failed to locate dependency: libcurl") endif() +if(NOT OPENSSL_FOUND) + message(FATAL_ERROR "Failed to locate dependency: OpenSSL") +else() + if(OPENSSL_VERSION VERSION_LESS 1 AND NOT FORCE_OPENSSL100) + add_definitions("-DOPENSSL_0_9") + else() + add_definitions("-DOPENSSL_1_0") + endif() +endif() + set(CMAKE_INSTALL_RPATH ${CMAKE_INSTALL_RPATH}:${CMAKE_INSTALL_PREFIX}/${MAXSCALE_LIBDIR}) # Make sure the release notes for this release are present if it is a stable one diff --git a/server/core/gateway.c b/server/core/gateway.c index f206ca30e..5997a9eeb 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -263,6 +263,15 @@ static void ssl_free_dynlock(struct CRYPTO_dynlock_value * n,const char* file, i free(n); } +/** + * The thread ID callback function for OpenSSL dynamic locks. + * @param id Id to modify + */ +static void maxscale_ssl_id(CRYPTO_THREADID* id) +{ + CRYPTO_THREADID_set_numeric(id,pthread_self()); +} + /** * Handler for SIGHUP signal. Reload the configuration for the * gateway. @@ -1459,8 +1468,11 @@ int main(int argc, char **argv) CRYPTO_set_dynlock_create_callback(ssl_create_dynlock); CRYPTO_set_dynlock_destroy_callback(ssl_free_dynlock); CRYPTO_set_dynlock_lock_callback(ssl_lock_dynlock); +#ifdef OPENSSL_1_0 + CRYPTO_THREADID_set_callback(maxscale_ssl_id); +#else CRYPTO_set_id_callback(pthread_self); - +#endif /* register exit function for embedded MySQL library */ l = atexit(libmysqld_done); From 0f199d924f937c81f057285b07ca2871d7df5aa9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 12:56:22 +0300 Subject: [PATCH 30/97] Removed unnecessary call to SSL_get_error from dcb_accept_SSL. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 4bd51d9a2..3597d0d3a 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -3010,7 +3010,7 @@ int dcb_accept_SSL(DCB* dcb) do { ssl_rval = SSL_accept(dcb->ssl); - errnum = SSL_get_error(dcb->ssl,ssl_rval); + LOGIF(LD,(skygw_log_write_flush(LD,"[dcb_accept_SSL] SSL_accept %d, error %d", ssl_rval,errnum))); switch(ssl_rval) From 067a62b24032e17dc1bae46bfba9eb0f95471496 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 14:34:26 +0300 Subject: [PATCH 31/97] Added more error logging to dcb_write_SSL. --- server/core/dcb.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 3597d0d3a..e63a73ffc 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1645,7 +1645,7 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) { char errbuf[140]; ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LE,"%s",errbuf); + skygw_log_write(LD,"%s",errbuf); } } break; @@ -1665,6 +1665,19 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) STRDCBSTATE(dcb->state), dcb->fd, ssl_errno))); + if(ssl_errno == SSL_ERROR_SSL) + { + while((ssl_errno = ERR_get_error()) != 0) + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE,"%s",errbuf); + } + } + if(ssl_errno == SSL_ERROR_SYSCALL) + { + skygw_log_write(LE,"%d:%s",errno,strerror(errno)); + } } } break; From 484781a463913f242bba6d370f3b43d734132007 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 14:46:46 +0300 Subject: [PATCH 32/97] More error logging for SSL connections. --- server/core/dcb.c | 47 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index e63a73ffc..4e2ee4800 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1659,7 +1659,7 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : Write to dcb %p in " - "state %s fd %d failed due " + "state %s fd %d failed due to " "SSL error %d", dcb, STRDCBSTATE(dcb->state), @@ -1881,28 +1881,47 @@ dcb_drain_writeq_SSL(DCB *dcb) while (dcb->writeq != NULL) { len = GWBUF_LENGTH(dcb->writeq); - w = gw_write_SSL(dcb->ssl, GWBUF_DATA(dcb->writeq), len); + w = gw_write_SSL(dcb->ssl, GWBUF_DATA(dcb->writeq), len); if (w < 0) { - int ssl_errno = ERR_get_error(); + int ssl_errno = SSL_get_error(dcb->ssl,w); - if(ssl_errno == SSL_ERROR_WANT_WRITE || - ssl_errno == SSL_ERROR_WANT_ACCEPT || - ssl_errno == SSL_ERROR_WANT_READ) + if(ssl_errno == SSL_ERROR_WANT_WRITE || ssl_errno == SSL_ERROR_WANT_READ) { break; } + skygw_log_write_flush(LOGFILE_ERROR, + "Error : Write to dcb failed due to " + "SSL error %d:", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ssl_errno); + switch(ssl_errno) + { + case SSL_ERROR_SSL: + case SSL_ERROR_SYSCALL: + while((ssl_errno = ERR_get_error()) != 0) + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE,"%s",errbuf); + } + if(errno != 0) + skygw_log_write(LE,"%d:%s",errno,strerror(errno)); + break; + case SSL_ERROR_ZERO_RETURN: + skygw_log_write(LE,"Socket is closed."); + break; - skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Write to dcb %p " - "in state %s fd %d failed: %s", - dcb, - STRDCBSTATE(dcb->state), - dcb->fd, - ERR_error_string(ssl_errno,NULL)); + default: + skygw_log_write(LE,"Unexpected error."); + break; + } break; + + } /* * Pull the number of bytes we have written from From 80d130ef0c6f9d65407519920d79dd6e5bbcaf14 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 15:05:04 +0300 Subject: [PATCH 33/97] Fixed dcb_write_SSL being called multiple times on failure. --- server/core/dcb.c | 182 +++++++++++++++++++++++++--------------------- 1 file changed, 101 insertions(+), 81 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 4e2ee4800..9142f7ee9 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1594,108 +1594,128 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) } #endif /* FAKE_CODE */ qlen = GWBUF_LENGTH(queue); - - w = gw_write_SSL(dcb->ssl, GWBUF_DATA(queue), qlen); - dcb->stats.n_writes++; - - if (w < 0) + do { - int ssl_errno = SSL_get_error(dcb->ssl,w); + w = gw_write_SSL(dcb->ssl, GWBUF_DATA(queue), qlen); + dcb->stats.n_writes++; - if (LOG_IS_ENABLED(LOGFILE_DEBUG)) + if (w <= 0) { - switch(ssl_errno) + int ssl_errno = SSL_get_error(dcb->ssl,w); + + if (LOG_IS_ENABLED(LOGFILE_DEBUG)) { - case SSL_ERROR_WANT_READ: - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_write] Write to dcb " - "%p in state %s fd %d failed " - "due error SSL_ERROR_WANT_READ", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state), - dcb->fd))); - break; - case SSL_ERROR_WANT_WRITE: - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_write] Write to dcb " - "%p in state %s fd %d failed " - "due error SSL_ERROR_WANT_WRITE", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state), - dcb->fd))); - break; - default: - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_write] Write to dcb " - "%p in state %s fd %d failed " - "due error %d", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state), - dcb->fd,ssl_errno))); - if(ssl_errno == SSL_ERROR_SSL || - ssl_errno == SSL_ERROR_SYSCALL) + switch(ssl_errno) { - while((ssl_errno = ERR_get_error()) != 0) + case SSL_ERROR_WANT_READ: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error SSL_ERROR_WANT_READ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + break; + case SSL_ERROR_WANT_WRITE: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error SSL_ERROR_WANT_WRITE", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd))); + break; + default: + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Write to dcb " + "%p in state %s fd %d failed " + "due error %d", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state), + dcb->fd,ssl_errno))); + if(ssl_errno == SSL_ERROR_SSL || + ssl_errno == SSL_ERROR_SYSCALL) { - char errbuf[140]; - ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LD,"%s",errbuf); + while((ssl_errno = ERR_get_error()) != 0) + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LD,"%s",errbuf); + } } - } - break; - } - } - - if (LOG_IS_ENABLED(LOGFILE_ERROR)) - { - if (ssl_errno != 0) - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Write to dcb %p in " - "state %s fd %d failed due to " - "SSL error %d", - dcb, - STRDCBSTATE(dcb->state), - dcb->fd, - ssl_errno))); - if(ssl_errno == SSL_ERROR_SSL) - { - while((ssl_errno = ERR_get_error()) != 0) - { - char errbuf[140]; - ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LE,"%s",errbuf); - } - } - if(ssl_errno == SSL_ERROR_SYSCALL) - { - skygw_log_write(LE,"%d:%s",errno,strerror(errno)); + break; } } + + if (LOG_IS_ENABLED(LOGFILE_ERROR) && ssl_errno != SSL_ERROR_WANT_WRITE) + { + if (ssl_errno == -1) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Write to dcb %p in " + "state %s fd %d failed due to " + "SSL error %d", + dcb, + STRDCBSTATE(dcb->state), + dcb->fd, + ssl_errno))); + if(ssl_errno == SSL_ERROR_SSL) + { + do + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE,"%s",errbuf); + }while((ssl_errno = ERR_get_error()) != 0); + } + if(ssl_errno == SSL_ERROR_SYSCALL) + { + skygw_log_write(LE,"%d:%s",errno,strerror(errno)); + } + } + else if(w == 0) + { + do + { + char errbuf[140]; + ERR_error_string(ssl_errno,errbuf); + skygw_log_write(LE,"%s",errbuf); + }while((ssl_errno = ERR_get_error()) != 0); + } + } + + if(ssl_errno != SSL_ERROR_WANT_WRITE) + break; } - break; - } + }while(w <= 0); /* * Pull the number of bytes we have written from * queue with have. */ - queue = gwbuf_consume(queue, w); - LOGIF(LD, (skygw_log_write( + if(w == -1) + { + while((queue = GWBUF_CONSUME_ALL(queue))); + } + else + { + queue = gwbuf_consume(queue, w); + LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [dcb_write] Wrote %d Bytes to dcb %p in " - "state %s fd %d", + "state %s fd %d", pthread_self(), w, dcb, STRDCBSTATE(dcb->state), dcb->fd))); + } } /*< while (queue != NULL) */ /*< * What wasn't successfully written is stored to write queue From 047985fb91a21c94fc6446ba1e26de33ae8aa74c Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 15:26:35 +0300 Subject: [PATCH 34/97] Fixed SSL thread locking functions not being used. --- server/core/gateway.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index 5997a9eeb..1b653d2f7 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -208,6 +208,15 @@ static int set_user(); /** SSL multi-threading functions and structures */ +static SPINLOCK* ssl_locks; + +static void ssl_locking_function(int mode,int n,const char* file, int line) +{ + if(mode & CRYPTO_LOCK) + spinlock_acquire(&ssl_locks[n]); + else + spinlock_release(&ssl_locks[n]); +} /** * OpenSSL requires this struct to be defined in order to use dynamic locks */ @@ -1465,14 +1474,19 @@ int main(int argc, char **argv) SSL_library_init(); SSL_load_error_strings(); OPENSSL_add_all_algorithms_noconf(); - CRYPTO_set_dynlock_create_callback(ssl_create_dynlock); - CRYPTO_set_dynlock_destroy_callback(ssl_free_dynlock); - CRYPTO_set_dynlock_lock_callback(ssl_lock_dynlock); -#ifdef OPENSSL_1_0 - CRYPTO_THREADID_set_callback(maxscale_ssl_id); -#else - CRYPTO_set_id_callback(pthread_self); -#endif + + int numlocks = CRYPTO_num_locks(); + if((ssl_locks = malloc(sizeof(SPINLOCK)*(numlocks + 1))) == NULL) + { + char* logerr = "Memory allocation failed"; + print_log_n_stderr(true, true, logerr, logerr, eno); + rc = MAXSCALE_INTERNALERROR; + goto return_main; + } + + for(i = 0;i Date: Wed, 24 Jun 2015 16:56:27 +0300 Subject: [PATCH 35/97] Moved SSL spinlock initialization to be done after thread initialization. --- server/core/gateway.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index 1b653d2f7..6513bdb1f 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -1475,18 +1475,6 @@ int main(int argc, char **argv) SSL_load_error_strings(); OPENSSL_add_all_algorithms_noconf(); - int numlocks = CRYPTO_num_locks(); - if((ssl_locks = malloc(sizeof(SPINLOCK)*(numlocks + 1))) == NULL) - { - char* logerr = "Memory allocation failed"; - print_log_n_stderr(true, true, logerr, logerr, eno); - rc = MAXSCALE_INTERNALERROR; - goto return_main; - } - - for(i = 0;i Date: Wed, 24 Jun 2015 18:17:00 +0300 Subject: [PATCH 36/97] Updated readwritesplit documentation. --- Documentation/routers/ReadWriteSplit.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/routers/ReadWriteSplit.md b/Documentation/routers/ReadWriteSplit.md index 6058e0390..577bc2e36 100644 --- a/Documentation/routers/ReadWriteSplit.md +++ b/Documentation/routers/ReadWriteSplit.md @@ -4,7 +4,9 @@ This document provides a short overview of the **readwritesplit** router module ## Overview -The **readwritesplit** router is designed to be used with a Master-Slave replication cluster. It automatically detects changes in the master server and will use the current master server of the cluster. With a Galera cluster, one can achieve a resilient setup and easy master failover by using one of the Galera nodes as a Write-Master node, where are write queries are routed, and spreading the read load over all the nodes. +The **readwritesplit** router is designed to increase the read-only processing capability of a cluster while maintaining consistency. This is achieved by splitting the query load into read and write queries. Read queries, which do not modify data, are spread across multiple nodes while all write queries will be sent to a single node. + +The router is designed to be used with a traditional Master-Slave replication cluster. It automatically detects changes in the master server and will use the current master server of the cluster. With a Galera cluster, one can achieve a resilient setup and easy master failover by using one of the Galera nodes as a Write-Master node, where all write queries are routed, and spreading the read load over all the nodes. ## Configuration From 7a8c30751587ff530e1ffff57e66530a2e235186 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 24 Jun 2015 19:23:43 +0300 Subject: [PATCH 37/97] MXS-171: https://mariadb.atlassian.net/browse/MXS-171 Added option which allows the master server to be used for reads. --- Documentation/routers/ReadWriteSplit.md | 7 +++++++ server/modules/include/readwritesplit.h | 1 + server/modules/routing/readwritesplit/readwritesplit.c | 7 ++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/routers/ReadWriteSplit.md b/Documentation/routers/ReadWriteSplit.md index 577bc2e36..e58367789 100644 --- a/Documentation/routers/ReadWriteSplit.md +++ b/Documentation/routers/ReadWriteSplit.md @@ -93,6 +93,13 @@ disable_sescmd_history=true disable_slave_recovery=true ``` +**`master_accept_reads`** allows the master server to be used for reads. This is a useful option to enable if you are using a small number of servers and wish to use the master for reads as well. + +``` +# Use the master for reads +master_accept_reads=true +``` + ## Limitations In Master-Slave replication cluster also read-only queries are routed to master too in the following situations: diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index 9aa55016f..c78cde5b4 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -252,6 +252,7 @@ typedef struct rwsplit_config_st { int rw_max_sescmd_history_size; bool disable_sescmd_hist; bool disable_slave_recovery; + bool master_reads; /*< Use master for reads */ } rwsplit_config_t; diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b9beb80d0..7b4a808c3 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1244,7 +1244,8 @@ static bool get_dcb( SERVER_IS_SLAVE(b->backend_server) && (max_rlag == MAX_RLAG_UNDEFINED || (b->backend_server->rlag != MAX_RLAG_NOT_AVAILABLE && - b->backend_server->rlag <= max_rlag))) + b->backend_server->rlag <= max_rlag)) && + !rses->rses_config.master_reads) { /** found slave */ candidate_bref = &backend_ref[i]; @@ -4589,6 +4590,10 @@ static void rwsplit_process_router_options( { router->rwsplit_config.disable_slave_recovery = config_truth_value(value); } + else if(strcmp(options[i],"master_accept_reads") == 0) + { + router->rwsplit_config.master_reads = config_truth_value(value); + } } } /*< for */ } From e5d9abbdcbdb9a31bc18c1e5ea5fb64b3fe8c75f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 25 Jun 2015 06:01:33 +0300 Subject: [PATCH 38/97] Fixes to Coverity defects. --- server/core/dcb.c | 82 +++++++----------------------- server/core/secrets.c | 6 +++ server/core/service.c | 2 +- server/modules/monitor/galeramon.c | 3 ++ utils/skygw_utils.cc | 2 + 5 files changed, 29 insertions(+), 66 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 9142f7ee9..8f2f159d4 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -939,10 +939,10 @@ int dcb_read_n( goto return_n; } - if (b == 0 && nread == 0) + if (b == 0) { /** Handle closed client socket */ - if (dcb_isclient(dcb)) + if (nread == 0 && dcb_isclient(dcb)) { char c; int l_errno = 0; @@ -964,11 +964,6 @@ int dcb_read_n( n = 0; goto return_n; } - else if (b == 0) - { - n = 0; - goto return_n; - } dcb->last_read = hkheartbeat; @@ -1208,6 +1203,10 @@ int dcb_read_SSL( } gwbuf_rtrim(buffer,bufsize - n); + if(buffer == NULL) + { + goto return_n; + } #ifdef SS_DEBUG skygw_log_write(LD,"%lu SSL: Truncated buffer from %d to %d bytes. " "Read %d bytes, %d bytes waiting.\n",pthread_self(), @@ -1234,9 +1233,6 @@ int dcb_read_SSL( /*< Append read data to the gwbuf */ *head = gwbuf_append(*head, buffer); - rc = ioctl(dcb->fd, FIONREAD, &b); - pending = SSL_pending(dcb->ssl); - } /*< while (true) */ return_n: return nread; @@ -1639,16 +1635,6 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) dcb, STRDCBSTATE(dcb->state), dcb->fd,ssl_errno))); - if(ssl_errno == SSL_ERROR_SSL || - ssl_errno == SSL_ERROR_SYSCALL) - { - while((ssl_errno = ERR_get_error()) != 0) - { - char errbuf[140]; - ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LD,"%s",errbuf); - } - } break; } } @@ -1666,19 +1652,19 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) STRDCBSTATE(dcb->state), dcb->fd, ssl_errno))); - if(ssl_errno == SSL_ERROR_SSL) + if(ssl_errno == SSL_ERROR_SSL || ssl_errno == SSL_ERROR_SYSCALL) { + if(ssl_errno == SSL_ERROR_SYSCALL) + { + skygw_log_write(LE,"%d:%s",errno,strerror(errno)); + } do { char errbuf[140]; ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LE,"%s",errbuf); + skygw_log_write(LE,"%d:%s",ssl_errno,errbuf); }while((ssl_errno = ERR_get_error()) != 0); } - if(ssl_errno == SSL_ERROR_SYSCALL) - { - skygw_log_write(LE,"%d:%s",errno,strerror(errno)); - } } else if(w == 0) { @@ -1686,7 +1672,7 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) { char errbuf[140]; ERR_error_string(ssl_errno,errbuf); - skygw_log_write(LE,"%s",errbuf); + skygw_log_write(LE,"%d:%s",ssl_errno,errbuf); }while((ssl_errno = ERR_get_error()) != 0); } } @@ -1695,16 +1681,14 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) break; } }while(w <= 0); - /* - * Pull the number of bytes we have written from - * queue with have. - */ - if(w == -1) + + if(w <= 0) { - while((queue = GWBUF_CONSUME_ALL(queue))); + break; } else { + /** Remove written bytes from the queue */ queue = gwbuf_consume(queue, w); LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, @@ -1733,38 +1717,6 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) } } /* if (dcb->writeq) */ - if (saved_errno != 0 && - queue != NULL && - saved_errno != EAGAIN && - saved_errno != EWOULDBLOCK) - { - bool dolog = true; - - /** - * Do not log if writing COM_QUIT to backend failed. - */ - if (GWBUF_IS_TYPE_MYSQL(queue)) - { - uint8_t* data = GWBUF_DATA(queue); - - if (data[4] == 0x01) - { - dolog = false; - } - } - if (dolog) - { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_write] Writing to %s socket failed due %d, %s.", - pthread_self(), - dcb_isclient(dcb) ? "client" : "backend server", - saved_errno, - strerror(saved_errno)))); - } - spinlock_release(&dcb->writeqlock); - return 0; - } spinlock_release(&dcb->writeqlock); if (dcb->high_water && dcb->writeqlen > dcb->high_water && below_water) diff --git a/server/core/secrets.c b/server/core/secrets.c index 97cb49806..b0e0c5542 100644 --- a/server/core/secrets.c +++ b/server/core/secrets.c @@ -228,6 +228,12 @@ unsigned int randval; MAXKEYS key; char secret_file[PATH_MAX + 10]; +if(strlen(path) > PATH_MAX) +{ + skygw_log_write(LOGFILE_ERROR,"Error: Pathname too long."); + return 1; +} + sprintf(secret_file,"%s/.secrets",path); /* Open for writing | Create | Truncate the file for writing */ diff --git a/server/core/service.c b/server/core/service.c index 24d51542d..be05932ae 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -268,7 +268,7 @@ GWPROTOCOL *funcs; /* Save authentication data to file cache */ char *ptr, path[PATH_MAX + 1]; int mkdir_rval = 0; - strcpy(path, get_cachedir()); + strncpy(path, get_cachedir(),PATH_MAX); strncat(path, "/", 4096); strncat(path, service->name, PATH_MAX); if (access(path, R_OK) == -1) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index 07d9fd848..06a6e6de8 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -155,7 +155,10 @@ startMonitor(void *arg,void* opt) else if(!strcmp(params->name,"script")) { if(handle->script) + { free(handle->script); + handle->script = NULL; + } if(access(params->value,X_OK) == 0) { diff --git a/utils/skygw_utils.cc b/utils/skygw_utils.cc index 4e3f84dca..bce8c631a 100644 --- a/utils/skygw_utils.cc +++ b/utils/skygw_utils.cc @@ -396,6 +396,7 @@ mlist_cursor_t* mlist_cursor_init( if (c == NULL) { goto return_cursor; + simple_mutex_unlock(&list->mlist_mutex); } c->mlcursor_chk_top = CHK_NUM_MLIST_CURSOR; c->mlcursor_chk_tail = CHK_NUM_MLIST_CURSOR; @@ -581,6 +582,7 @@ bool mlist_cursor_move_to_first( simple_mutex_lock(&list->mlist_mutex, true); if (mc->mlcursor_list->mlist_deleted) { + simple_mutex_unlock(&list->mlist_mutex); return false; } /** Set position point to first node */ From 61bee570d17404ff84735068fff3ec664d59ca41 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 25 Jun 2015 11:56:27 +0300 Subject: [PATCH 39/97] Fixed build failures due to old OpenSSL libraries. --- server/core/gateway.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/core/gateway.c b/server/core/gateway.c index 6513bdb1f..c474083ee 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -272,6 +272,7 @@ static void ssl_free_dynlock(struct CRYPTO_dynlock_value * n,const char* file, i free(n); } +#ifdef OPENSSL_1_0 /** * The thread ID callback function for OpenSSL dynamic locks. * @param id Id to modify @@ -280,6 +281,7 @@ static void maxscale_ssl_id(CRYPTO_THREADID* id) { CRYPTO_THREADID_set_numeric(id,pthread_self()); } +#endif /** * Handler for SIGHUP signal. Reload the configuration for the From 80709ce039c3b07ec7cbc5461648d071be55f277 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 25 Jun 2015 16:43:50 +0300 Subject: [PATCH 40/97] Fixed compile errors for older SSL libraries. --- server/core/service.c | 3 ++- server/include/service.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/server/core/service.c b/server/core/service.c index be05932ae..9822ac881 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -1966,13 +1966,14 @@ int serviceInitSSL(SERVICE* service) case SERVICE_TLS10: service->method = (SSL_METHOD*)TLSv1_server_method(); break; +#ifdef OPENSSL_1_0 case SERVICE_TLS11: service->method = (SSL_METHOD*)TLSv1_1_server_method(); break; case SERVICE_TLS12: service->method = (SSL_METHOD*)TLSv1_2_server_method(); break; - +#endif /** Rest of these use the maximum available SSL/TLS methods */ case SERVICE_SSL_MAX: service->method = (SSL_METHOD*)SSLv23_server_method(); diff --git a/server/include/service.h b/server/include/service.h index 3337ebfc0..b64270b9e 100644 --- a/server/include/service.h +++ b/server/include/service.h @@ -117,8 +117,10 @@ typedef enum { enum{ SERVICE_SSLV3, SERVICE_TLS10, +#ifdef OPENSSL_1_0 SERVICE_TLS11, SERVICE_TLS12, +#endif SERVICE_SSL_MAX, SERVICE_TLS_MAX, SERVICE_SSL_TLS_MAX From cff01cad055a29a6116ea5d2de106a2aaa6eb38e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 25 Jun 2015 17:13:10 +0300 Subject: [PATCH 41/97] Fixed compile errors with old SSL library. --- server/core/service.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/core/service.c b/server/core/service.c index 9822ac881..3dee01c91 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -923,10 +923,12 @@ serviceSetSSLVersion(SERVICE *service, char* version) service->ssl_method_type = SERVICE_SSLV3; else if(strcasecmp(version,"TLSV10") == 0) service->ssl_method_type = SERVICE_TLS10; +#ifdef OPENSSL_1_0 else if(strcasecmp(version,"TLSV11") == 0) service->ssl_method_type = SERVICE_TLS11; else if(strcasecmp(version,"TLSV12") == 0) service->ssl_method_type = SERVICE_TLS12; +#endif else if(strcasecmp(version,"MAX") == 0) service->ssl_method_type = SERVICE_SSL_TLS_MAX; else return -1; From 22053cfafbd3c933617b49505d3d5d5130768e09 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 26 Jun 2015 10:36:27 +0300 Subject: [PATCH 42/97] Updated MaxScale version to 1.2.0. --- cmake/macros.cmake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/macros.cmake b/cmake/macros.cmake index c287343fb..b78502cf9 100644 --- a/cmake/macros.cmake +++ b/cmake/macros.cmake @@ -9,13 +9,13 @@ macro(set_maxscale_version) # MaxScale version number set(MAXSCALE_VERSION_MAJOR "1") - set(MAXSCALE_VERSION_MINOR "1") - set(MAXSCALE_VERSION_PATCH "1") + set(MAXSCALE_VERSION_MINOR "2") + set(MAXSCALE_VERSION_PATCH "0") set(MAXSCALE_VERSION_NUMERIC "${MAXSCALE_VERSION_MAJOR}.${MAXSCALE_VERSION_MINOR}.${MAXSCALE_VERSION_PATCH}") set(MAXSCALE_VERSION "${MAXSCALE_VERSION_MAJOR}.${MAXSCALE_VERSION_MINOR}.${MAXSCALE_VERSION_PATCH}") # This should be incremented each time a package is rebuilt - set(MAXSCALE_BUILD_NUMBER 2) + set(MAXSCALE_BUILD_NUMBER 1) endmacro() macro(set_variables) From fc1615c48931b7aab5fd255fcdae5f7e55f73eed Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 26 Jun 2015 13:35:14 +0300 Subject: [PATCH 43/97] Fixed the test header configuration failing. --- server/test/maxscale_test.h.in | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/test/maxscale_test.h.in b/server/test/maxscale_test.h.in index d41ff181f..b5448295c 100644 --- a/server/test/maxscale_test.h.in +++ b/server/test/maxscale_test.h.in @@ -1,9 +1,9 @@ #ifndef MAXSCALE_TEST_H #define MAXSCALE_TEST_H -#define TEST_DIR "${CMAKE_BINARY_DIR}" -#define TEST_LOG_DIR "${CMAKE_BINARY_DIR}/log" -#define TEST_BIN_DIR "${CMAKE_BINARY_DIR}/bin" -#define TEST_MOD_DIR "${CMAKE_BINARY_DIR}/modules" -#define TEST_LIB_DIR "${CMAKE_BINARY_DIR}/lib" -#define TEST_ETC_DIR "${CMAKE_BINARY_DIR}/etc" +#define TEST_DIR "@CMAKE_BINARY_DIR@" +#define TEST_LOG_DIR "@CMAKE_BINARY_DIR@/log" +#define TEST_BIN_DIR "@CMAKE_BINARY_DIR@/bin" +#define TEST_MOD_DIR "@CMAKE_BINARY_DIR@/modules" +#define TEST_LIB_DIR "@CMAKE_BINARY_DIR@/lib" +#define TEST_ETC_DIR "@CMAKE_BINARY_DIR@/etc" #endif From 03503a8f9bfc1a6e2da8ce5062fc59f9b5409f81 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Fri, 26 Jun 2015 17:31:40 +0100 Subject: [PATCH 44/97] Fix a number of relatively simple bugs shown by Coverity. --- log_manager/log_manager.cc | 4 +-- server/core/config.c | 2 +- .../modules/routing/maxinfo/maxinfo_parse.c | 4 --- .../routing/readwritesplit/readwritesplit.c | 27 ++++++++++--------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index b31ae6112..a87157be7 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -1394,7 +1394,7 @@ int skygw_log_write_flush( /** * Write log string to buffer and add to file write list. */ - for(i = LOGFILE_FIRST;i<=LOGFILE_LAST;i <<=1) + for (i = LOGFILE_FIRST; irses_lock)); + myrses = *rses; + ss_dassert(SPINLOCK_IS_LOCKED(&myrses->rses_lock)); ses = backend_dcb->session; CHK_SESSION(ses); @@ -4782,7 +4785,7 @@ static bool handle_error_new_connection( /** * If bref == NULL it has been replaced already with another one. */ - if ((bref = get_bref_from_dcb(rses, backend_dcb)) == NULL) + if ((bref = get_bref_from_dcb(myrses, backend_dcb)) == NULL) { succp = true; goto return_succp; @@ -4825,25 +4828,25 @@ static bool handle_error_new_connection( (void *)bref); router_nservers = router_get_servercount(inst); - max_nslaves = rses_get_max_slavecount(rses, router_nservers); - max_slave_rlag = rses_get_max_replication_lag(rses); + max_nslaves = rses_get_max_slavecount(myrses, router_nservers); + max_slave_rlag = rses_get_max_replication_lag(myrses); /** * Try to get replacement slave or at least the minimum * number of slave connections for router session. */ if(inst->rwsplit_config.disable_slave_recovery) { - succp = have_enough_servers(&rses,1,router_nservers,inst) ? true : false; + succp = have_enough_servers(&myrses,1,router_nservers,inst) ? true : false; } else { succp = select_connect_backend_servers( - &rses->rses_master_ref, - rses->rses_backend_ref, + &myrses->rses_master_ref, + myrses->rses_backend_ref, router_nservers, max_nslaves, max_slave_rlag, - rses->rses_config.rw_slave_select_criteria, + myrses->rses_config.rw_slave_select_criteria, ses, inst); } From c9606e1071fb6f21413002d7fa5d7ff78e810b46 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 27 Jun 2015 08:28:37 +0300 Subject: [PATCH 45/97] Fixed build failures due to mismatching function prototypes. --- server/modules/routing/readwritesplit/readwritesplit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 45ce0ab5f..70554c544 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -287,7 +287,7 @@ static sescmd_cursor_t* backend_ref_get_sescmd_cursor (backend_ref_t* bref); static int router_handle_state_switch(DCB* dcb, DCB_REASON reason, void* data); static bool handle_error_new_connection( ROUTER_INSTANCE* inst, - ROUTER_CLIENT_SES* rses, + ROUTER_CLIENT_SES** rses, DCB* backend_dcb, GWBUF* errmsg); static void handle_error_reply_client( From 4759df9f87723b1002ccba1a95f3447394bf861d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 27 Jun 2015 09:08:20 +0300 Subject: [PATCH 46/97] Added proper wildcard database grant detection. --- server/core/dbusers.c | 35 ++++++++++++++++++++++++++ server/modules/protocol/mysql_common.c | 7 ------ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index f4e0ed080..a54ed73a7 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -1706,6 +1706,41 @@ static int uh_cmpfun( void* v1, void* v2) { return 0; } + if(hu2->resource && strlen(hu2->resource) && strchr(hu2->resource,'%') != NULL) + { + regex_t re; + char db[MYSQL_DATABASE_MAXLEN*2 +1]; + strcpy(db,hu2->resource); + int len = strlen(db); + char* ptr = strrchr(db,'%'); + + if(ptr == NULL) + { + return 1; + } + + while(ptr) + { + memmove(ptr+1,ptr,(len - (ptr - db)) + 1); + *ptr = '.'; + *(ptr + 1) = '*'; + len = strlen(db); + ptr = strrchr(db,'%'); + } + + if((regcomp(&re,db,REG_ICASE|REG_NOSUB))) + { + return 1; + } + + if(regexec(&re,hu1->resource,0,NULL,0) == 0) + { + regfree(&re); + return 0; + } + regfree(&re); + } + /* no matches, deny auth */ return 1; } diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 83e2912e5..9b138360a 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1552,13 +1552,6 @@ int gw_find_mysql_user_password_sha1(char *username, uint8_t *gateway_password, break; } - /** See if ANYDB == Y */ - if(key.resource) - { - key.resource = NULL; - continue; - } - if (!user_password) { /* * user@% not found. From 88940c0097d4f76f04e53472c59f8b86b5a481d3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 27 Jun 2015 09:47:06 +0300 Subject: [PATCH 47/97] Fixed wildcard grants not being added to the users table. --- server/core/dbusers.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index a54ed73a7..935644c18 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -974,18 +974,10 @@ getAllUsers(SERVICE *service, USERS *users) } } - if(havedb && wildcard_db_grant(dbnm)) + if(havedb && wildcard_db_grant(dbnm) && service->optimize_wildcard) { - if(service->optimize_wildcard) - { - rc = add_wildcard_users(users, row[0], row[1], password, row[4], dbnm, service->resources); - skygw_log_write(LOGFILE_DEBUG|LOGFILE_TRACE,"%s: Converted '%s' to %d individual database grants.",service->name,dbnm,rc); - } - else - { - /** Use ANYDB for wildcard grants */ - rc = add_mysql_users_with_host_ipv4(users, row[0], row[1], password, "Y", NULL); - } + rc = add_wildcard_users(users, row[0], row[1], password, row[4], dbnm, service->resources); + skygw_log_write(LOGFILE_DEBUG|LOGFILE_TRACE,"%s: Converted '%s' to %d individual database grants.",service->name,dbnm,rc); } else { @@ -1041,8 +1033,8 @@ getAllUsers(SERVICE *service, USERS *users) } else if(rc == -1) { /** Duplicate user*/ - LOGIF(LE,(skygw_log_write(LT|LE, - "Warning: Duplicate MySQL user found for service [%s]: %s@%s%s%s", + LOGIF(LT,(skygw_log_write(LT, + "Duplicate MySQL user found for service [%s]: %s@%s%s%s", service->name, row[0],row[1],havedb?" for database: ":"", havedb ?dbnm:""))); From 05991384a6f7ac9bef6d9d04ab165cb392dcfd41 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 27 Jun 2015 09:51:59 +0300 Subject: [PATCH 48/97] Added missing removal of systemd files to the postinstall script. --- etc/postrm.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etc/postrm.in b/etc/postrm.in index 65c0f7116..7488ebe46 100755 --- a/etc/postrm.in +++ b/etc/postrm.in @@ -3,7 +3,9 @@ if [ "$1" -eq 0 ] then rm -f /etc/init.d/maxscale rm -f /etc/ld.so.conf.d/maxscale.conf + rm -f /usr/lib/systemd/system/maxscale.service else + # Copy and rename config from old location if [ -f "/usr/local/mariadb-maxscale/etc/MaxScale.cnf" ] then cp "/usr/local/mariadb-maxscale/etc/MaxScale.cnf" "/etc/maxscale.cnf" From 113fb4c33bb6ee07c13821b8472b2e797e95ceff Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 28 Jun 2015 08:43:05 +0300 Subject: [PATCH 49/97] Fix to MXS-209: https://mariadb.atlassian.net/browse/MXS-209 Added missing checks for proper column count on query result. --- server/modules/monitor/galeramon.c | 15 +++++++++++ server/modules/monitor/mmmon.c | 35 +++++++++++++++++++++++--- server/modules/monitor/mysql_mon.c | 23 +++++++++++++++++ server/modules/monitor/ndbclustermon.c | 14 +++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index 06a6e6de8..e1ddc1217 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -372,6 +372,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'wsrep_local_state'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_state'\""); + return; + } + while ((row = mysql_fetch_row(result))) { if (strcmp(row[1], "4") == 0) @@ -398,6 +405,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long local_index = -1; + + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_index'\""); + return; + } + while ((row = mysql_fetch_row(result))) { local_index = strtol(row[1], NULL, 10); diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 545a77ca2..970fd0d31 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -366,7 +366,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long server_id = -1; - + + if(mysql_field_count(database->con) != 1) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + return; + } + while ((row = mysql_fetch_row(result))) { server_id = strtol(row[0], NULL, 10); @@ -392,7 +399,15 @@ char *server_string; { int i = 0; long master_id = -1; - + + if(mysql_field_count(database->con) < 42) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -431,7 +446,15 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long master_id = -1; - + + if(mysql_field_count(database->con) < 40) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW SLAVE STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -463,6 +486,12 @@ char *server_string; if (mysql_query(database->con, "SHOW GLOBAL VARIABLES LIKE 'read_only'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\""); + return; + } while ((row = mysql_fetch_row(result))) { diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index e310e2ede..4c5f57ab6 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -408,6 +408,12 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long server_id = -1; + if(mysql_field_count(database->con) != 1) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + return; + } while ((row = mysql_fetch_row(result))) { server_id = strtol(row[0], NULL, 10); @@ -433,6 +439,15 @@ char *server_string; { int i = 0; long master_id = -1; + + if(mysql_field_count(database->con) < 42) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ @@ -471,6 +486,14 @@ char *server_string; && (result = mysql_store_result(database->con)) != NULL) { long master_id = -1; + if(mysql_field_count(database->con) < 40) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: SHOW SLAVE STATUS " + "returned less than the expected amount of rows."); + return; + } + while ((row = mysql_fetch_row(result))) { /* get Slave_IO_Running and Slave_SQL_Running values*/ diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index 9561e275e..ed218d43e 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -323,6 +323,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\""); + return; + } + while ((row = mysql_fetch_row(result))) { if (atoi(row[1]) > 0) @@ -335,6 +342,13 @@ char *server_string; if (mysql_query(database->con, "SHOW STATUS LIKE 'Ndb_cluster_node_id'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\""); + return; + } + long cluster_node_id = -1; while ((row = mysql_fetch_row(result))) { From 5c7a30e9fe326dc11af5c525581b771692c867c2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 28 Jun 2015 10:43:06 +0300 Subject: [PATCH 50/97] Added more error logging. --- server/modules/monitor/galeramon.c | 13 +++++++++++-- server/modules/monitor/mmmon.c | 26 ++++++++++++++++++++------ server/modules/monitor/mysql_mon.c | 22 +++++++++++++++++----- server/modules/monitor/ndbclustermon.c | 6 ++++-- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index e1ddc1217..f520b44aa 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -375,7 +375,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_state'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'wsrep_local_state'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } @@ -389,6 +390,13 @@ char *server_string; if (mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0 && (result = mysql_store_result(database->con)) != NULL) { + if(mysql_field_count(database->con) < 2) + { + mysql_free_result(result); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW VARIABLES LIKE 'wsrep_sst_method'\". Expected 2 columns." + " MySQL Version: %s",version_str); + return; + } while ((row = mysql_fetch_row(result))) { if (strncmp(row[1], "xtrabackup", 10) == 0) @@ -409,7 +417,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'wsrep_local_index'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'wsrep_local_index'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 970fd0d31..e70b3684a 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -370,7 +370,8 @@ char *server_string; if(mysql_field_count(database->con) != 1) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + skygw_log_write(LE,"Error: Unexpected result for 'SELECT @@server_id'. Expected 1 column." + " MySQL Version: %s",version_str); return; } @@ -403,8 +404,9 @@ char *server_string; if(mysql_field_count(database->con) < 42) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " - "returned less than the expected amount of rows."); + skygw_log_write(LE,"Error: \"SHOW ALL SLAVES STATUS\" " + "returned less than the expected amount of columns. Expected 42 columns" + " MySQL Version: %s",version_str); return; } @@ -450,8 +452,19 @@ char *server_string; if(mysql_field_count(database->con) < 40) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW SLAVE STATUS " - "returned less than the expected amount of rows."); + + if(server_version < 5*10000 + 5*100) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." + " MySQL Version: %s",version_str); + } + else + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + "returned less than the expected amount of columns. Expected 40 columns." + " MySQL Version: %s",version_str); + } return; } @@ -489,7 +502,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW GLOBAL VARIABLES LIKE 'read_only'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index 4c5f57ab6..d2fa7acbd 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -411,7 +411,8 @@ char *server_string; if(mysql_field_count(database->con) != 1) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for 'SELECT @@server_id'."); + skygw_log_write(LE,"Error: Unexpected result for \"SELECT @@server_id\". Expected 1 columns." + " MySQL Version: %s",version_str); return; } while ((row = mysql_fetch_row(result))) @@ -443,8 +444,9 @@ char *server_string; if(mysql_field_count(database->con) < 42) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW ALL SLAVES STATUS " - "returned less than the expected amount of rows."); + skygw_log_write(LE,"Error: \"SHOW ALL SLAVES STATUS\" " + "returned less than the expected amount of columns. Expected 42 columns." + " MySQL Version: %s",version_str); return; } @@ -489,8 +491,18 @@ char *server_string; if(mysql_field_count(database->con) < 40) { mysql_free_result(result); - skygw_log_write(LE,"Error: SHOW SLAVE STATUS " - "returned less than the expected amount of rows."); + if(server_version < 5*10000 + 5*100) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." + " MySQL Version: %s",version_str); + } + else + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + "returned less than the expected amount of columns. Expected 40 columns." + " MySQL Version: %s",version_str); + } return; } diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index ed218d43e..a5d0b2455 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -326,7 +326,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'Ndb_number_of_ready_data_nodes'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } @@ -345,7 +346,8 @@ char *server_string; if(mysql_field_count(database->con) < 2) { mysql_free_result(result); - skygw_log_write(LE,"Error: Malformed result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\""); + skygw_log_write(LE,"Error: Unexpected result for \"SHOW STATUS LIKE 'Ndb_cluster_node_id'\". Expected 2 columns." + " MySQL Version: %s",version_str); return; } From 0062d9d2b783b752ee18a89fda6241a2c31c4ef9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 10:24:16 +0300 Subject: [PATCH 51/97] Version errors for SHOW SLAVE STATUS now only print once. --- server/core/monitor.c | 1 + server/include/monitor.h | 1 + server/modules/monitor/mmmon.c | 11 ++++++++--- server/modules/monitor/mysql_mon.c | 11 ++++++++--- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/server/core/monitor.c b/server/core/monitor.c index 0f24e691d..f422b4f6b 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -192,6 +192,7 @@ monitorAddServer(MONITOR *mon, SERVER *server) db->con = NULL; db->next = NULL; db->mon_err_count = 0; + db->log_version_err = true; /** Server status is uninitialized */ db->mon_prev_status = -1; /* pending status is updated by get_replication_tree */ diff --git a/server/include/monitor.h b/server/include/monitor.h index 681f34bac..442efb27f 100644 --- a/server/include/monitor.h +++ b/server/include/monitor.h @@ -124,6 +124,7 @@ typedef enum typedef struct monitor_servers { SERVER *server; /**< The server being monitored */ MYSQL *con; /**< The MySQL connection */ + bool log_version_err; int mon_err_count; unsigned int mon_prev_status; unsigned int pending_status; /**< Pending Status flag bitmap */ diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index e70b3684a..40c2c11fb 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -455,9 +455,14 @@ char *server_string; if(server_version < 5*10000 + 5*100) { - skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " - " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." - " MySQL Version: %s",version_str); + if(database->log_version_err) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for versions less than 5.5 does not have master_server_id, " + "replication tree cannot be resolved for server %s." + " MySQL Version: %s",database->server->unique_name,version_str); + database->log_version_err = false; + } } else { diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index d2fa7acbd..2ba70a499 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -493,9 +493,14 @@ char *server_string; mysql_free_result(result); if(server_version < 5*10000 + 5*100) { - skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " - " for MySQL 5.1 does not have master_server_id, replication tree cannot be resolved." - " MySQL Version: %s",version_str); + if(database->log_version_err) + { + skygw_log_write(LE,"Error: \"SHOW SLAVE STATUS\" " + " for versions less than 5.5 does not have master_server_id, " + "replication tree cannot be resolved for server %s." + " MySQL Version: %s",database->server->unique_name,version_str); + database->log_version_err = false; + } } else { From 2c3c856ea1b781b0e61f21836bf4a4bd0434f336 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 16:01:56 +0300 Subject: [PATCH 52/97] Added option for C99 builds. --- CMakeLists.txt | 8 +++++++- cmake/macros.cmake | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c28b2c657..62abd54a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,13 @@ if(PROFILE) set(FLAGS "${FLAGS} -pg " CACHE STRING "Compilation flags" FORCE) endif() -set(CMAKE_C_FLAGS "${FLAGS}") +if(USE_C99) + message(STATUS "Using C99 standard") + set(CMAKE_C_FLAGS "-std=c99 -D_GNU_SOURCE=1 ${FLAGS}") +else() + set(CMAKE_C_FLAGS "${FLAGS}") +endif() + set(CMAKE_C_FLAGS_DEBUG "${DEBUG_FLAGS} -DSS_DEBUG -DLOG_ASSERT") set(CMAKE_C_FLAGS_RELEASE "") set(CMAKE_C_FLAGS_RELWITHDEBINFO "-ggdb") diff --git a/cmake/macros.cmake b/cmake/macros.cmake index b78502cf9..d7921f1c4 100644 --- a/cmake/macros.cmake +++ b/cmake/macros.cmake @@ -20,6 +20,9 @@ endmacro() macro(set_variables) + # Use C99 + set(USE_C99 FALSE CACHE BOOL "Use C99 standard") + # hostname or IP address of MaxScale's host set(TEST_HOST "127.0.0.1" CACHE STRING "hostname or IP address of MaxScale's host") From 6f343ff57b7c599d0825c4094dfc6afb05ce140f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 29 Jun 2015 19:17:12 +0300 Subject: [PATCH 53/97] Fix to MXS-227: https://mariadb.atlassian.net/browse/MXS-227 Fixed memory leak. --- server/modules/monitor/galeramon.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index f520b44aa..11f9c565a 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -280,7 +280,7 @@ monitorDatabase(MONITOR *mon, MONITOR_SERVERS *database) { GALERA_MONITOR* handle = (GALERA_MONITOR*)mon->handle; MYSQL_ROW row; -MYSQL_RES *result; +MYSQL_RES *result,*result2; int isjoined = 0; char *uname = mon->user; char *passwd = mon->password; @@ -388,11 +388,12 @@ char *server_string; /* Check if the node is a donor and is using xtrabackup, in this case it can stay alive */ else if (strcmp(row[1], "2") == 0 && handle->availableWhenDonor == 1) { if (mysql_query(database->con, "SHOW VARIABLES LIKE 'wsrep_sst_method'") == 0 - && (result = mysql_store_result(database->con)) != NULL) + && (result2 = mysql_store_result(database->con)) != NULL) { if(mysql_field_count(database->con) < 2) { mysql_free_result(result); + mysql_free_result(result2); skygw_log_write(LE,"Error: Unexpected result for \"SHOW VARIABLES LIKE 'wsrep_sst_method'\". Expected 2 columns." " MySQL Version: %s",version_str); return; @@ -402,6 +403,7 @@ char *server_string; if (strncmp(row[1], "xtrabackup", 10) == 0) isjoined = 1; } + mysql_free_result(result2); } } } From a2e281823a6f6796b85a458f3a25927745747c4c Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 14:58:36 +0300 Subject: [PATCH 54/97] Added NULL checks to readwritesplit. --- .../routing/readwritesplit/readwritesplit.c | 131 +++++++++++++----- 1 file changed, 99 insertions(+), 32 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 14f99c361..b5b4ddb36 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -226,7 +226,7 @@ static rses_property_t* mysql_sescmd_get_property( static rses_property_t* rses_property_init( rses_property_type_t prop_type); -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop); @@ -2943,6 +2943,11 @@ static void bref_clear_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_clear_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state &= ~state; @@ -2972,6 +2977,11 @@ static void bref_set_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_set_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state |= state; @@ -3535,7 +3545,8 @@ static rses_property_t* rses_property_init( prop = (rses_property_t*)calloc(1, sizeof(rses_property_t)); if (prop == NULL) { - goto return_prop; + skygw_log_write(LE,"Error: Malloc returned NULL. (%s:%d)",__FILE__,__LINE__); + return NULL; } prop->rses_prop_type = prop_type; #if defined(SS_DEBUG) @@ -3554,6 +3565,11 @@ return_prop: static void rses_property_done( rses_property_t* prop) { + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(prop); switch (prop->rses_prop_type) { @@ -3587,10 +3603,20 @@ static void rses_property_done( * * Router client session must be locked. */ -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop) { + if(rses == NULL) + { + skygw_log_write(LE,"Error: Router client session is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } + if(prop == NULL) + { + skygw_log_write(LE,"Error: Router client session property is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } rses_property_t* p; CHK_CLIENT_RSES(rses); @@ -3612,6 +3638,7 @@ static void rses_property_add( } p->rses_prop_next = prop; } + return 0; } /** @@ -3622,7 +3649,13 @@ static mysql_sescmd_t* rses_property_get_sescmd( rses_property_t* prop) { mysql_sescmd_t* sescmd; - + + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); + return; + } + CHK_RSES_PROP(prop); ss_dassert(prop->rses_prop_rsession == NULL || SPINLOCK_IS_LOCKED(&prop->rses_prop_rsession->rses_lock)); @@ -3635,22 +3668,6 @@ static mysql_sescmd_t* rses_property_get_sescmd( } return sescmd; } - -/** -static void rses_begin_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_acquire(&prop->rses_prop_lock); -} - -static void rses_end_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_release(&prop->rses_prop_lock); -} -*/ /** * Create session command property. @@ -3683,6 +3700,11 @@ static mysql_sescmd_t* mysql_sescmd_init ( static void mysql_sescmd_done( mysql_sescmd_t* sescmd) { + if(sescmd == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to mysql_sescmd_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(sescmd->my_sescmd_prop); gwbuf_free(sescmd->my_sescmd_buf); memset(sescmd, 0, sizeof(mysql_sescmd_t)); @@ -3855,6 +3877,12 @@ static bool sescmd_cursor_is_active( sescmd_cursor_t* sescmd_cursor) { bool succp; + + if(sescmd_cursor == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_is_active. (%s:%d)",__FILE__,__LINE__); + return false; + } ss_dassert(SPINLOCK_IS_LOCKED(&sescmd_cursor->scmd_cur_rses->rses_lock)); succp = sescmd_cursor->scmd_cur_active; @@ -3880,6 +3908,11 @@ static GWBUF* sescmd_cursor_clone_querybuf( sescmd_cursor_t* scur) { GWBUF* buf; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_clone_querybuf. (%s:%d)",__FILE__,__LINE__); + return NULL; + } ss_dassert(scur->scmd_cur_cmd != NULL); buf = gwbuf_clone(scur->scmd_cur_cmd->my_sescmd_buf); @@ -3892,7 +3925,12 @@ static bool sescmd_cursor_history_empty( sescmd_cursor_t* scur) { bool succp; - + + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_history_empty. (%s:%d)",__FILE__,__LINE__); + return true; + } CHK_SESCMD_CUR(scur); if (scur->scmd_cur_rses->rses_properties[RSES_PROP_TYPE_SESCMD] == NULL) @@ -3912,6 +3950,11 @@ static void sescmd_cursor_reset( sescmd_cursor_t* scur) { ROUTER_CLIENT_SES* rses; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_reset. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_SESCMD_CUR(scur); CHK_CLIENT_RSES(scur->scmd_cur_rses); rses = scur->scmd_cur_rses; @@ -3928,6 +3971,11 @@ static bool execute_sescmd_history( { bool succp; sescmd_cursor_t* scur; + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_history. (%s:%d)",__FILE__,__LINE__); + return false; + } CHK_BACKEND_REF(bref); scur = &bref->bref_sescmd_cur; @@ -3964,6 +4012,11 @@ static bool execute_sescmd_in_backend( int rc = 0; sescmd_cursor_t* scur; + if(backend_ref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_in_backend. (%s:%d)",__FILE__,__LINE__); + return false; + } if (BREF_IS_CLOSED(backend_ref)) { succp = false; @@ -4084,6 +4137,12 @@ static bool sescmd_cursor_next( rses_property_t* prop_curr; rses_property_t* prop_next; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_next. (%s:%d)",__FILE__,__LINE__); + return false; + } + ss_dassert(scur != NULL); ss_dassert(*(scur->scmd_cur_ptr_property) != NULL); ss_dassert(SPINLOCK_IS_LOCKED( @@ -4410,11 +4469,21 @@ static bool route_session_write( * prevent it from being released before properties * are cleaned up as a part of router sessionclean-up. */ - prop = rses_property_init(RSES_PROP_TYPE_SESCMD); + if((prop = rses_property_init(RSES_PROP_TYPE_SESCMD)) == NULL) + { + skygw_log_write(LE,"Error: Router session property initialization failed"); + rses_end_locked_router_action(router_cli_ses); + return false; + } mysql_sescmd_init(prop, querybuf, packet_type, router_cli_ses); /** Add sescmd property to router client session */ - rses_property_add(router_cli_ses, prop); + if(rses_property_add(router_cli_ses, prop) != 0) + { + skygw_log_write(LE,"Error: Session property addition failed."); + rses_end_locked_router_action(router_cli_ses); + return false; + } for (i=0; irses_nbackends; i++) { @@ -4537,7 +4606,10 @@ static void rwsplit_process_router_options( int i; char* value; select_criteria_t c; - + + if(options == NULL) + return; + for (i = 0; options[i]; i++) { if ((value = strchr(options[i], '=')) == NULL) @@ -4625,7 +4697,7 @@ static void handleError ( SESSION* session; ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - + CHK_DCB(backend_dcb); /** Reset error handle flag from a given DCB */ @@ -5088,8 +5160,6 @@ static int router_handle_state_switch( { backend_ref_t* bref; int rc = 1; - ROUTER_CLIENT_SES* rses; - SESSION* ses; SERVER* srv; CHK_DCB(dcb); @@ -5110,11 +5180,8 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - ses = dcb->session; - CHK_SESSION(ses); - - rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; - CHK_CLIENT_RSES(rses); + CHK_SESSION(dcb->session); + CHK_CLIENT_RSES(dcb->session->router_session); switch (reason) { case DCB_REASON_NOT_RESPONDING: From 4706b756c25c62b89803cc5bdf1bafa0521c1d57 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 19:16:49 +0300 Subject: [PATCH 55/97] Changed the service resource hashing function into a proper string hashing function. --- server/core/dbusers.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 935644c18..6748a1437 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -1842,18 +1842,6 @@ char *mysql_format_user_entry(void *data) return mysql_user; } -/* - * The hash function we use for storing MySQL database names. - * - * @param key The key value - * @return The hash key - */ -int -resource_hash(char *key) -{ - return (*key + *(key + 1)); -} - /** * Remove the resources table * @@ -1877,7 +1865,7 @@ resource_alloc() { HASHTABLE *resources; - if ((resources = hashtable_alloc(10, resource_hash, strcmp)) == NULL) + if ((resources = hashtable_alloc(10, simple_str_hash, strcmp)) == NULL) { return NULL; } From e350f19e6f07b0f3912d10a5937d8133cd2b94b9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 14:58:36 +0300 Subject: [PATCH 56/97] Added NULL checks to readwritesplit. --- .../routing/readwritesplit/readwritesplit.c | 131 +++++++++++++----- 1 file changed, 99 insertions(+), 32 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 14f99c361..b5b4ddb36 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -226,7 +226,7 @@ static rses_property_t* mysql_sescmd_get_property( static rses_property_t* rses_property_init( rses_property_type_t prop_type); -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop); @@ -2943,6 +2943,11 @@ static void bref_clear_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_clear_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state &= ~state; @@ -2972,6 +2977,11 @@ static void bref_set_state( backend_ref_t* bref, bref_state_t state) { + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to bref_set_state. (%s:%d)",__FILE__,__LINE__); + return; + } if (state != BREF_WAITING_RESULT) { bref->bref_state |= state; @@ -3535,7 +3545,8 @@ static rses_property_t* rses_property_init( prop = (rses_property_t*)calloc(1, sizeof(rses_property_t)); if (prop == NULL) { - goto return_prop; + skygw_log_write(LE,"Error: Malloc returned NULL. (%s:%d)",__FILE__,__LINE__); + return NULL; } prop->rses_prop_type = prop_type; #if defined(SS_DEBUG) @@ -3554,6 +3565,11 @@ return_prop: static void rses_property_done( rses_property_t* prop) { + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(prop); switch (prop->rses_prop_type) { @@ -3587,10 +3603,20 @@ static void rses_property_done( * * Router client session must be locked. */ -static void rses_property_add( +static int rses_property_add( ROUTER_CLIENT_SES* rses, rses_property_t* prop) { + if(rses == NULL) + { + skygw_log_write(LE,"Error: Router client session is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } + if(prop == NULL) + { + skygw_log_write(LE,"Error: Router client session property is NULL. (%s:%d)",__FILE__,__LINE__); + return -1; + } rses_property_t* p; CHK_CLIENT_RSES(rses); @@ -3612,6 +3638,7 @@ static void rses_property_add( } p->rses_prop_next = prop; } + return 0; } /** @@ -3622,7 +3649,13 @@ static mysql_sescmd_t* rses_property_get_sescmd( rses_property_t* prop) { mysql_sescmd_t* sescmd; - + + if(prop == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); + return; + } + CHK_RSES_PROP(prop); ss_dassert(prop->rses_prop_rsession == NULL || SPINLOCK_IS_LOCKED(&prop->rses_prop_rsession->rses_lock)); @@ -3635,22 +3668,6 @@ static mysql_sescmd_t* rses_property_get_sescmd( } return sescmd; } - -/** -static void rses_begin_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_acquire(&prop->rses_prop_lock); -} - -static void rses_end_locked_property_action( - rses_property_t* prop) -{ - CHK_RSES_PROP(prop); - spinlock_release(&prop->rses_prop_lock); -} -*/ /** * Create session command property. @@ -3683,6 +3700,11 @@ static mysql_sescmd_t* mysql_sescmd_init ( static void mysql_sescmd_done( mysql_sescmd_t* sescmd) { + if(sescmd == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to mysql_sescmd_done. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_RSES_PROP(sescmd->my_sescmd_prop); gwbuf_free(sescmd->my_sescmd_buf); memset(sescmd, 0, sizeof(mysql_sescmd_t)); @@ -3855,6 +3877,12 @@ static bool sescmd_cursor_is_active( sescmd_cursor_t* sescmd_cursor) { bool succp; + + if(sescmd_cursor == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_is_active. (%s:%d)",__FILE__,__LINE__); + return false; + } ss_dassert(SPINLOCK_IS_LOCKED(&sescmd_cursor->scmd_cur_rses->rses_lock)); succp = sescmd_cursor->scmd_cur_active; @@ -3880,6 +3908,11 @@ static GWBUF* sescmd_cursor_clone_querybuf( sescmd_cursor_t* scur) { GWBUF* buf; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_clone_querybuf. (%s:%d)",__FILE__,__LINE__); + return NULL; + } ss_dassert(scur->scmd_cur_cmd != NULL); buf = gwbuf_clone(scur->scmd_cur_cmd->my_sescmd_buf); @@ -3892,7 +3925,12 @@ static bool sescmd_cursor_history_empty( sescmd_cursor_t* scur) { bool succp; - + + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_history_empty. (%s:%d)",__FILE__,__LINE__); + return true; + } CHK_SESCMD_CUR(scur); if (scur->scmd_cur_rses->rses_properties[RSES_PROP_TYPE_SESCMD] == NULL) @@ -3912,6 +3950,11 @@ static void sescmd_cursor_reset( sescmd_cursor_t* scur) { ROUTER_CLIENT_SES* rses; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_reset. (%s:%d)",__FILE__,__LINE__); + return; + } CHK_SESCMD_CUR(scur); CHK_CLIENT_RSES(scur->scmd_cur_rses); rses = scur->scmd_cur_rses; @@ -3928,6 +3971,11 @@ static bool execute_sescmd_history( { bool succp; sescmd_cursor_t* scur; + if(bref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_history. (%s:%d)",__FILE__,__LINE__); + return false; + } CHK_BACKEND_REF(bref); scur = &bref->bref_sescmd_cur; @@ -3964,6 +4012,11 @@ static bool execute_sescmd_in_backend( int rc = 0; sescmd_cursor_t* scur; + if(backend_ref == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_in_backend. (%s:%d)",__FILE__,__LINE__); + return false; + } if (BREF_IS_CLOSED(backend_ref)) { succp = false; @@ -4084,6 +4137,12 @@ static bool sescmd_cursor_next( rses_property_t* prop_curr; rses_property_t* prop_next; + if(scur == NULL) + { + skygw_log_write(LE,"Error: NULL parameter passed to sescmd_cursor_next. (%s:%d)",__FILE__,__LINE__); + return false; + } + ss_dassert(scur != NULL); ss_dassert(*(scur->scmd_cur_ptr_property) != NULL); ss_dassert(SPINLOCK_IS_LOCKED( @@ -4410,11 +4469,21 @@ static bool route_session_write( * prevent it from being released before properties * are cleaned up as a part of router sessionclean-up. */ - prop = rses_property_init(RSES_PROP_TYPE_SESCMD); + if((prop = rses_property_init(RSES_PROP_TYPE_SESCMD)) == NULL) + { + skygw_log_write(LE,"Error: Router session property initialization failed"); + rses_end_locked_router_action(router_cli_ses); + return false; + } mysql_sescmd_init(prop, querybuf, packet_type, router_cli_ses); /** Add sescmd property to router client session */ - rses_property_add(router_cli_ses, prop); + if(rses_property_add(router_cli_ses, prop) != 0) + { + skygw_log_write(LE,"Error: Session property addition failed."); + rses_end_locked_router_action(router_cli_ses); + return false; + } for (i=0; irses_nbackends; i++) { @@ -4537,7 +4606,10 @@ static void rwsplit_process_router_options( int i; char* value; select_criteria_t c; - + + if(options == NULL) + return; + for (i = 0; options[i]; i++) { if ((value = strchr(options[i], '=')) == NULL) @@ -4625,7 +4697,7 @@ static void handleError ( SESSION* session; ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - + CHK_DCB(backend_dcb); /** Reset error handle flag from a given DCB */ @@ -5088,8 +5160,6 @@ static int router_handle_state_switch( { backend_ref_t* bref; int rc = 1; - ROUTER_CLIENT_SES* rses; - SESSION* ses; SERVER* srv; CHK_DCB(dcb); @@ -5110,11 +5180,8 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - ses = dcb->session; - CHK_SESSION(ses); - - rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; - CHK_CLIENT_RSES(rses); + CHK_SESSION(dcb->session); + CHK_CLIENT_RSES(dcb->session->router_session); switch (reason) { case DCB_REASON_NOT_RESPONDING: From a2e31d6846321c15a127f5cc9e372e9f1dc02759 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 30 Jun 2015 19:16:49 +0300 Subject: [PATCH 57/97] Changed the service resource hashing function into a proper string hashing function. --- server/core/dbusers.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 935644c18..6748a1437 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -1842,18 +1842,6 @@ char *mysql_format_user_entry(void *data) return mysql_user; } -/* - * The hash function we use for storing MySQL database names. - * - * @param key The key value - * @return The hash key - */ -int -resource_hash(char *key) -{ - return (*key + *(key + 1)); -} - /** * Remove the resources table * @@ -1877,7 +1865,7 @@ resource_alloc() { HASHTABLE *resources; - if ((resources = hashtable_alloc(10, resource_hash, strcmp)) == NULL) + if ((resources = hashtable_alloc(10, simple_str_hash, strcmp)) == NULL) { return NULL; } From 6c61dabfab706b654637e166aecc49966194fd50 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 05:15:08 +0300 Subject: [PATCH 58/97] Fix to a Coverity defect. --- server/modules/routing/readwritesplit/readwritesplit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b5b4ddb36..c4008e65e 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3653,7 +3653,7 @@ static mysql_sescmd_t* rses_property_get_sescmd( if(prop == NULL) { skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); - return; + return NULL; } CHK_RSES_PROP(prop); From cc24777a902f85966bd23ce73748a66f73ad02a2 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Wed, 1 Jul 2015 09:46:01 +0100 Subject: [PATCH 59/97] Correct mkdir logic for default log directory. --- server/core/gateway.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index c474083ee..8f486067c 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -1531,17 +1531,13 @@ int main(int argc, char **argv) /** Use default log directory /var/log/maxscale/ */ if(logdir == NULL) { - - if(access(default_logdir,F_OK) != 0) - { - if(mkdir(logdir,0555) != 0) - { - fprintf(stderr, - "Error: Cannot create log directory: %s\n", - default_logdir); - goto return_main; - } - } + if(mkdir(default_logdir,0777) != 0 && errno != EEXIST) + { + fprintf(stderr, + "Error: Cannot create log directory: %s\n", + default_logdir); + goto return_main; + } logdir = strdup(default_logdir); } @@ -1598,7 +1594,7 @@ int main(int argc, char **argv) /** * Set a data directory for the mysqld library, we use * a unique directory name to avoid clauses if multiple - * instances of the gateway are beign run on the same + * instances of the gateway are being run on the same * machine. */ From 53a4dd393d0ad4b77be9f1c50525abcd5f315e01 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 05:15:08 +0300 Subject: [PATCH 60/97] Fix to a Coverity defect. --- server/modules/routing/readwritesplit/readwritesplit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b5b4ddb36..c4008e65e 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3653,7 +3653,7 @@ static mysql_sescmd_t* rses_property_get_sescmd( if(prop == NULL) { skygw_log_write(LE,"Error: NULL parameter passed to rses_property_get_sescmd. (%s:%d)",__FILE__,__LINE__); - return; + return NULL; } CHK_RSES_PROP(prop); From dc15fbb576ce19cca92f309f6c572238242353be Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 16:46:32 +0300 Subject: [PATCH 61/97] Fixed build failure. --- server/modules/routing/readwritesplit/readwritesplit.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c4008e65e..b801d9aec 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -5161,7 +5161,8 @@ static int router_handle_state_switch( backend_ref_t* bref; int rc = 1; SERVER* srv; - + ROUTER_CLIENT_SES* rses; + SESSION* ses; CHK_DCB(dcb); bref = (backend_ref_t *)data; CHK_BACKEND_REF(bref); @@ -5180,8 +5181,10 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - CHK_SESSION(dcb->session); - CHK_CLIENT_RSES(dcb->session->router_session); + ses = dcb->session; + CHK_SESSION(ses); + rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; + CHK_CLIENT_RSES(rses); switch (reason) { case DCB_REASON_NOT_RESPONDING: From 22f8e61321a9c5bad6c7c02aca8c1e1687666113 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 1 Jul 2015 16:46:32 +0300 Subject: [PATCH 62/97] Fixed build failure. --- server/modules/routing/readwritesplit/readwritesplit.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c4008e65e..b801d9aec 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -5161,7 +5161,8 @@ static int router_handle_state_switch( backend_ref_t* bref; int rc = 1; SERVER* srv; - + ROUTER_CLIENT_SES* rses; + SESSION* ses; CHK_DCB(dcb); bref = (backend_ref_t *)data; CHK_BACKEND_REF(bref); @@ -5180,8 +5181,10 @@ static int router_handle_state_switch( srv->name, srv->port, STRSRVSTATUS(srv)))); - CHK_SESSION(dcb->session); - CHK_CLIENT_RSES(dcb->session->router_session); + ses = dcb->session; + CHK_SESSION(ses); + rses = (ROUTER_CLIENT_SES *)dcb->session->router_session; + CHK_CLIENT_RSES(rses); switch (reason) { case DCB_REASON_NOT_RESPONDING: From ea14839dbcdc31ddd572e13e78eb895d63c505fd Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 2 Jul 2015 14:55:43 +0300 Subject: [PATCH 63/97] Updated release notes with fixed bugs. --- .../MaxScale-1.2.0-Release-Notes.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index a84f94c27..49206f8ed 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -29,3 +29,27 @@ MaxScale now supports SSL/TLS encrypted connections to MaxScale. ### Monitor scripts State changes in backend servers can now trigger the execution of a custom script. With this you can easily customize MaxScale's behavior. + +## Bug fixes + +Here is a list of bugs fixed since the release of MaxScale 1.1.1. +|Key|Summary| +|[MXS-24](https://mariadb.atlassian.net/browse/MXS-24)|bugzillaId-604: Module load path documentation issues ...| +|[MXS-40](https://mariadb.atlassian.net/browse/MXS-40)|Display logged in users| +|[MXS-113](https://mariadb.atlassian.net/browse/MXS-113)|MaxScale seems to fail if built against MariaDB 10.0 libraries| +|[MXS-116](https://mariadb.atlassian.net/browse/MXS-116)|Do not run maxscale as root.| +|[MXS-125](https://mariadb.atlassian.net/browse/MXS-125)|inconsistency in maxkeys/maxpassword output and parameters| +|[MXS-136](https://mariadb.atlassian.net/browse/MXS-136)|Check for MaxScale replication heartbeat table existence before creating| +|[MXS-137](https://mariadb.atlassian.net/browse/MXS-137)|cannot get sql for queries with length >= 0x80| +|[MXS-139](https://mariadb.atlassian.net/browse/MXS-139)|Schemarouter authentication for wildcard grants fails without optimize_wildcard| +|[MXS-140](https://mariadb.atlassian.net/browse/MXS-140)|strip_db_esc does not work without auth_all_servers| +|[MXS-162](https://mariadb.atlassian.net/browse/MXS-162)|Fix Incorrect info in Configuration Guide| +|[MXS-163](https://mariadb.atlassian.net/browse/MXS-163)|Reload config leaks memory| +|[MXS-165](https://mariadb.atlassian.net/browse/MXS-165)|Concurrency issue while incrementing sessions in qlafilter| +|[MXS-166](https://mariadb.atlassian.net/browse/MXS-166)|Memory leak when creating a new event| +|[MXS-171](https://mariadb.atlassian.net/browse/MXS-171)|Allow reads on master for readwritesplit| +|[MXS-176](https://mariadb.atlassian.net/browse/MXS-176)|Missing dependencies in documentation| +|[MXS-181](https://mariadb.atlassian.net/browse/MXS-181)|Poor performance on TCP connection due to Nagle's algoritm| +|[MXS-212](https://mariadb.atlassian.net/browse/MXS-212)|Stopped services accept connections| +|[MXS-225](https://mariadb.atlassian.net/browse/MXS-225)|RPM Debug build packages have no debugging symbols| +|[MXS-227](https://mariadb.atlassian.net/browse/MXS-227)|Memory leak in Galera Monitor| From 3817b6fca8e6df12c72617c35cb669b97dc1d084 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 2 Jul 2015 15:01:16 +0300 Subject: [PATCH 64/97] Fixed broken table. --- Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index 49206f8ed..f98c2191f 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -34,6 +34,7 @@ State changes in backend servers can now trigger the execution of a custom scrip Here is a list of bugs fixed since the release of MaxScale 1.1.1. |Key|Summary| +------------ |[MXS-24](https://mariadb.atlassian.net/browse/MXS-24)|bugzillaId-604: Module load path documentation issues ...| |[MXS-40](https://mariadb.atlassian.net/browse/MXS-40)|Display logged in users| |[MXS-113](https://mariadb.atlassian.net/browse/MXS-113)|MaxScale seems to fail if built against MariaDB 10.0 libraries| From fed2cd9a3a3df2a8a3aa79df9af9f5965047d83c Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 2 Jul 2015 15:02:19 +0300 Subject: [PATCH 65/97] More broken tables. --- Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index f98c2191f..636d46ad8 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -34,7 +34,7 @@ State changes in backend servers can now trigger the execution of a custom scrip Here is a list of bugs fixed since the release of MaxScale 1.1.1. |Key|Summary| ------------- +|---|--------| |[MXS-24](https://mariadb.atlassian.net/browse/MXS-24)|bugzillaId-604: Module load path documentation issues ...| |[MXS-40](https://mariadb.atlassian.net/browse/MXS-40)|Display logged in users| |[MXS-113](https://mariadb.atlassian.net/browse/MXS-113)|MaxScale seems to fail if built against MariaDB 10.0 libraries| From e09e5381ec9dc91080ff34828e717c23b8f50018 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 2 Jul 2015 15:05:03 +0300 Subject: [PATCH 66/97] Fixed table. --- .../MaxScale-1.2.0-Release-Notes.md | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index 636d46ad8..caf7e28c6 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -33,24 +33,25 @@ State changes in backend servers can now trigger the execution of a custom scrip ## Bug fixes Here is a list of bugs fixed since the release of MaxScale 1.1.1. -|Key|Summary| -|---|--------| -|[MXS-24](https://mariadb.atlassian.net/browse/MXS-24)|bugzillaId-604: Module load path documentation issues ...| -|[MXS-40](https://mariadb.atlassian.net/browse/MXS-40)|Display logged in users| -|[MXS-113](https://mariadb.atlassian.net/browse/MXS-113)|MaxScale seems to fail if built against MariaDB 10.0 libraries| -|[MXS-116](https://mariadb.atlassian.net/browse/MXS-116)|Do not run maxscale as root.| -|[MXS-125](https://mariadb.atlassian.net/browse/MXS-125)|inconsistency in maxkeys/maxpassword output and parameters| -|[MXS-136](https://mariadb.atlassian.net/browse/MXS-136)|Check for MaxScale replication heartbeat table existence before creating| -|[MXS-137](https://mariadb.atlassian.net/browse/MXS-137)|cannot get sql for queries with length >= 0x80| -|[MXS-139](https://mariadb.atlassian.net/browse/MXS-139)|Schemarouter authentication for wildcard grants fails without optimize_wildcard| -|[MXS-140](https://mariadb.atlassian.net/browse/MXS-140)|strip_db_esc does not work without auth_all_servers| -|[MXS-162](https://mariadb.atlassian.net/browse/MXS-162)|Fix Incorrect info in Configuration Guide| -|[MXS-163](https://mariadb.atlassian.net/browse/MXS-163)|Reload config leaks memory| -|[MXS-165](https://mariadb.atlassian.net/browse/MXS-165)|Concurrency issue while incrementing sessions in qlafilter| -|[MXS-166](https://mariadb.atlassian.net/browse/MXS-166)|Memory leak when creating a new event| -|[MXS-171](https://mariadb.atlassian.net/browse/MXS-171)|Allow reads on master for readwritesplit| -|[MXS-176](https://mariadb.atlassian.net/browse/MXS-176)|Missing dependencies in documentation| -|[MXS-181](https://mariadb.atlassian.net/browse/MXS-181)|Poor performance on TCP connection due to Nagle's algoritm| -|[MXS-212](https://mariadb.atlassian.net/browse/MXS-212)|Stopped services accept connections| -|[MXS-225](https://mariadb.atlassian.net/browse/MXS-225)|RPM Debug build packages have no debugging symbols| -|[MXS-227](https://mariadb.atlassian.net/browse/MXS-227)|Memory leak in Galera Monitor| + +| Key | Summary | +| --- | -------- | +| [MXS-24](https://mariadb.atlassian.net/browse/MXS-24) | bugzillaId-604: Module load path documentation issues ... | +| [MXS-40](https://mariadb.atlassian.net/browse/MXS-40) | Display logged in users | +| [MXS-113](https://mariadb.atlassian.net/browse/MXS-113) | MaxScale seems to fail if built against MariaDB 10.0 libraries | +| [MXS-116](https://mariadb.atlassian.net/browse/MXS-116) | Do not run maxscale as root. | +| [MXS-125](https://mariadb.atlassian.net/browse/MXS-125) | inconsistency in maxkeys/maxpassword output and parameters | +| [MXS-136](https://mariadb.atlassian.net/browse/MXS-136) | Check for MaxScale replication heartbeat table existence before creating | +| [MXS-137](https://mariadb.atlassian.net/browse/MXS-137) | cannot get sql for queries with length >= 0x80 | +| [MXS-139](https://mariadb.atlassian.net/browse/MXS-139) | Schemarouter authentication for wildcard grants fails without optimize_wildcard | +| [MXS-140](https://mariadb.atlassian.net/browse/MXS-140) | strip_db_esc does not work without auth_all_servers | +| [MXS-162](https://mariadb.atlassian.net/browse/MXS-162) | Fix Incorrect info in Configuration Guide | +| [MXS-163](https://mariadb.atlassian.net/browse/MXS-163) | Reload config leaks memory | +| [MXS-165](https://mariadb.atlassian.net/browse/MXS-165) | Concurrency issue while incrementing sessions in qlafilter | +| [MXS-166](https://mariadb.atlassian.net/browse/MXS-166) | Memory leak when creating a new event | +| [MXS-171](https://mariadb.atlassian.net/browse/MXS-171) | Allow reads on master for readwritesplit | +| [MXS-176](https://mariadb.atlassian.net/browse/MXS-176) | Missing dependencies in documentation | +| [MXS-181](https://mariadb.atlassian.net/browse/MXS-181) | Poor performance on TCP connection due to Nagle's algoritm | +| [MXS-212](https://mariadb.atlassian.net/browse/MXS-212) | Stopped services accept connections | +| [MXS-225](https://mariadb.atlassian.net/browse/MXS-225) | RPM Debug build packages have no debugging symbols | +| [MXS-227](https://mariadb.atlassian.net/browse/MXS-227) | Memory leak in Galera Monitor | From 1eb907ad38ef0e41265f90c61498df3e996f05f7 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 3 Jul 2015 17:08:32 +0300 Subject: [PATCH 67/97] Fixed common sharding functions referring to schemarouter. --- server/modules/routing/schemarouter/sharding_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/schemarouter/sharding_common.c b/server/modules/routing/schemarouter/sharding_common.c index d69b12d02..cf6bf2c12 100644 --- a/server/modules/routing/schemarouter/sharding_common.c +++ b/server/modules/routing/schemarouter/sharding_common.c @@ -46,7 +46,7 @@ bool extract_database(GWBUF* buf, char* str) tok = strtok_r(query," ;",&saved); if(tok == NULL || strcasecmp(tok,"use") != 0) { - skygw_log_write(LOGFILE_ERROR,"Schemarouter: Malformed chage database packet."); + skygw_log_write(LOGFILE_ERROR,"extract_database: Malformed chage database packet."); succp = false; goto retblock; } @@ -54,7 +54,7 @@ bool extract_database(GWBUF* buf, char* str) tok = strtok_r(NULL," ;",&saved); if(tok == NULL) { - skygw_log_write(LOGFILE_ERROR,"Schemarouter: Malformed chage database packet."); + skygw_log_write(LOGFILE_ERROR,"extract_database: Malformed chage database packet."); succp = false; goto retblock; } From 7e704d2312bda792183e3a124eb3e78fb9a0a4bd Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 07:13:14 +0300 Subject: [PATCH 68/97] Fixed module load failure referring to lib/maxscale/modules instead of lib/maxscale. --- server/core/service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/service.c b/server/core/service.c index 3dee01c91..4e36708f8 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -125,7 +125,7 @@ SERVICE *service; "Error : Unable to load %s module \"%s\".\n\t\t\t" " Ensure that lib%s.so exists in one of the " "following directories :\n\t\t\t " - "- %s/modules\n%s%s", + "- %s\n%s%s", MODULE_ROUTER, router, router, From 4f421cb69a89ba2f9e3f9bdd65897f8b7ec08abd Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 07:30:23 +0300 Subject: [PATCH 69/97] Added missing --piddir option to testall maxscale invokation. --- cmake/testall.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/testall.cmake b/cmake/testall.cmake index 65c27fa5a..839029312 100644 --- a/cmake/testall.cmake +++ b/cmake/testall.cmake @@ -1,4 +1,4 @@ -execute_process(COMMAND /bin/sh -c "${CMAKE_BINARY_DIR}/bin/maxscale -f ${CMAKE_BINARY_DIR}/maxscale.cnf --logdir=${CMAKE_BINARY_DIR}/ --datadir=${CMAKE_BINARY_DIR}/ --cachedir=${CMAKE_BINARY_DIR}/ &> ${CMAKE_BINARY_DIR}/maxscale.output" +execute_process(COMMAND /bin/sh -c "${CMAKE_BINARY_DIR}/bin/maxscale -f ${CMAKE_BINARY_DIR}/maxscale.cnf --logdir=${CMAKE_BINARY_DIR}/ --datadir=${CMAKE_BINARY_DIR}/ --cachedir=${CMAKE_BINARY_DIR}/ --piddir=${CMAKE_BINARY_DIR}/ &> ${CMAKE_BINARY_DIR}/maxscale.output" OUTPUT_VARIABLE MAXSCALE_OUT) execute_process(COMMAND make test RESULT_VARIABLE RVAL) execute_process(COMMAND killall maxscale) From 03b5a634f44f78d494ec7e4ee1ffb3c5fdaf3359 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 09:04:53 +0300 Subject: [PATCH 70/97] Fix to MXS-244: https://mariadb.atlassian.net/browse/MXS-244 Fixed buffers being only partially consumed if they contained more than one packet when a replu to a prepared statement is received. --- server/modules/routing/readwritesplit/readwritesplit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b801d9aec..3c329f646 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3787,7 +3787,7 @@ static GWBUF* sescmd_cursor_process_replies( dcb_close(bref->bref_dcb); *reconnect = true; if(replybuf) - gwbuf_consume(replybuf,gwbuf_length(replybuf)); + while((replybuf = gwbuf_consume(replybuf,gwbuf_length(replybuf)))); } } /** This is a response from the master and it is the "right" one. @@ -3830,7 +3830,7 @@ static GWBUF* sescmd_cursor_process_replies( skygw_log_write(LOGFILE_DEBUG,"Slave '%s' responded faster to a session command.", bref->bref_backend->backend_server->unique_name); if(replybuf) - gwbuf_free(replybuf); + while((replybuf = gwbuf_consume(replybuf,gwbuf_length(replybuf)))); return NULL; } From 5b75443744ef0a9b105efd8e40c017fd1ddeee44 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 07:13:14 +0300 Subject: [PATCH 71/97] Fixed module load failure referring to lib/maxscale/modules instead of lib/maxscale. --- server/core/service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/service.c b/server/core/service.c index 3dee01c91..4e36708f8 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -125,7 +125,7 @@ SERVICE *service; "Error : Unable to load %s module \"%s\".\n\t\t\t" " Ensure that lib%s.so exists in one of the " "following directories :\n\t\t\t " - "- %s/modules\n%s%s", + "- %s\n%s%s", MODULE_ROUTER, router, router, From 309b8bba9789de0036a48848ba4c347c717e44ca Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 07:30:23 +0300 Subject: [PATCH 72/97] Added missing --piddir option to testall maxscale invokation. --- cmake/testall.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/testall.cmake b/cmake/testall.cmake index 65c27fa5a..839029312 100644 --- a/cmake/testall.cmake +++ b/cmake/testall.cmake @@ -1,4 +1,4 @@ -execute_process(COMMAND /bin/sh -c "${CMAKE_BINARY_DIR}/bin/maxscale -f ${CMAKE_BINARY_DIR}/maxscale.cnf --logdir=${CMAKE_BINARY_DIR}/ --datadir=${CMAKE_BINARY_DIR}/ --cachedir=${CMAKE_BINARY_DIR}/ &> ${CMAKE_BINARY_DIR}/maxscale.output" +execute_process(COMMAND /bin/sh -c "${CMAKE_BINARY_DIR}/bin/maxscale -f ${CMAKE_BINARY_DIR}/maxscale.cnf --logdir=${CMAKE_BINARY_DIR}/ --datadir=${CMAKE_BINARY_DIR}/ --cachedir=${CMAKE_BINARY_DIR}/ --piddir=${CMAKE_BINARY_DIR}/ &> ${CMAKE_BINARY_DIR}/maxscale.output" OUTPUT_VARIABLE MAXSCALE_OUT) execute_process(COMMAND make test RESULT_VARIABLE RVAL) execute_process(COMMAND killall maxscale) From dc646bfbf16bf21978fedec789278379aca9f7c5 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 4 Jul 2015 09:04:53 +0300 Subject: [PATCH 73/97] Fix to MXS-244: https://mariadb.atlassian.net/browse/MXS-244 Fixed buffers being only partially consumed if they contained more than one packet when a replu to a prepared statement is received. --- server/modules/routing/readwritesplit/readwritesplit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index b801d9aec..3c329f646 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3787,7 +3787,7 @@ static GWBUF* sescmd_cursor_process_replies( dcb_close(bref->bref_dcb); *reconnect = true; if(replybuf) - gwbuf_consume(replybuf,gwbuf_length(replybuf)); + while((replybuf = gwbuf_consume(replybuf,gwbuf_length(replybuf)))); } } /** This is a response from the master and it is the "right" one. @@ -3830,7 +3830,7 @@ static GWBUF* sescmd_cursor_process_replies( skygw_log_write(LOGFILE_DEBUG,"Slave '%s' responded faster to a session command.", bref->bref_backend->backend_server->unique_name); if(replybuf) - gwbuf_free(replybuf); + while((replybuf = gwbuf_consume(replybuf,gwbuf_length(replybuf)))); return NULL; } From 5b95e2b32e30b9b27155f1b68243f9265e023d3b Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 6 Jul 2015 10:38:03 +0300 Subject: [PATCH 74/97] Updated 1.2 release notes. --- .../MaxScale-1.2.0-Release-Notes.md | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index caf7e28c6..de4b7a461 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -27,8 +27,8 @@ A quick list of changes in installation directories and file names: ### Client side SSL encryption MaxScale now supports SSL/TLS encrypted connections to MaxScale. -### Monitor scripts -State changes in backend servers can now trigger the execution of a custom script. With this you can easily customize MaxScale's behavior. +### Launchable scripts +Now you can configure MaxScale monitor module to automatically launch a script when it detects change in the state of a backend server. The script can be any customer script defined by you to take diagnostic or reporting action. With this you can easily customize MaxScale's behavior. ## Bug fixes @@ -40,18 +40,58 @@ Here is a list of bugs fixed since the release of MaxScale 1.1.1. | [MXS-40](https://mariadb.atlassian.net/browse/MXS-40) | Display logged in users | | [MXS-113](https://mariadb.atlassian.net/browse/MXS-113) | MaxScale seems to fail if built against MariaDB 10.0 libraries | | [MXS-116](https://mariadb.atlassian.net/browse/MXS-116) | Do not run maxscale as root. | +| [MXS-117](https://mariadb.atlassian.net/browse/MXS-117) | Allow configuration of the log file directory | | [MXS-125](https://mariadb.atlassian.net/browse/MXS-125) | inconsistency in maxkeys/maxpassword output and parameters | +| [MXS-128](https://mariadb.atlassian.net/browse/MXS-128) | cyclic dependency utils -> log_manager -> utils | | [MXS-136](https://mariadb.atlassian.net/browse/MXS-136) | Check for MaxScale replication heartbeat table existence before creating | | [MXS-137](https://mariadb.atlassian.net/browse/MXS-137) | cannot get sql for queries with length >= 0x80 | | [MXS-139](https://mariadb.atlassian.net/browse/MXS-139) | Schemarouter authentication for wildcard grants fails without optimize_wildcard | | [MXS-140](https://mariadb.atlassian.net/browse/MXS-140) | strip_db_esc does not work without auth_all_servers | -| [MXS-162](https://mariadb.atlassian.net/browse/MXS-162) | Fix Incorrect info in Configuration Guide | -| [MXS-163](https://mariadb.atlassian.net/browse/MXS-163) | Reload config leaks memory | +| [MXS-162](https://mariadb.atlassian.net/browse/MXS-162) | Fix Incorrect info in Configuration Guide | | [MXS-165](https://mariadb.atlassian.net/browse/MXS-165) | Concurrency issue while incrementing sessions in qlafilter | | [MXS-166](https://mariadb.atlassian.net/browse/MXS-166) | Memory leak when creating a new event | | [MXS-171](https://mariadb.atlassian.net/browse/MXS-171) | Allow reads on master for readwritesplit | | [MXS-176](https://mariadb.atlassian.net/browse/MXS-176) | Missing dependencies in documentation | +| [MXS-180](https://mariadb.atlassian.net/browse/MXS-180) | MariaDB10 binlog router compatibilty | | [MXS-181](https://mariadb.atlassian.net/browse/MXS-181) | Poor performance on TCP connection due to Nagle's algoritm | +| [MXS-182](https://mariadb.atlassian.net/browse/MXS-182) | SHOW SLAVE STATUS and maxadmin "show services" for binlog router needs updated when used with MariaDB 10 Master | | [MXS-212](https://mariadb.atlassian.net/browse/MXS-212) | Stopped services accept connections | | [MXS-225](https://mariadb.atlassian.net/browse/MXS-225) | RPM Debug build packages have no debugging symbols | | [MXS-227](https://mariadb.atlassian.net/browse/MXS-227) | Memory leak in Galera Monitor | +| [MXS-244](https://mariadb.atlassian.net/browse/MXS-244) | Memory leak when using prepared statements without arguments | + +## Known Issues and Limitations + +There are a number bugs and known limitations within this version of MaxScale, the most serious of this are listed below. + +* MaxScale can not manage authentication that uses wildcard matching in hostnames in the mysql.user table of the backend database. The only wildcards that can be used are in IP address entries. + +* When users have different passwords based on the host from which they connect MaxScale is unable to determine which password it should use to connect to the backend database. This results in failed connections and unusable usernames in MaxScale. + +* LONGBLOB are currently not supported. + +* Galera Cluster variables, such as @@wsrep_node_name, are not resolved by the embedded MariaDB parser. + +* The Database Firewall filter does not support multi-statements. Using them will result in an error being sent to the client. + +## Packaging + +Both RPM and Debian packages are available for MaxScale in addition to the tar based releases previously distributed we now provide + +* CentOS/RedHat 5 + +* CentOS/RedHat 6 + +* CentOS/RedHat 7 + +* Debian 6 + +* Debian 7 + +* Ubuntu 12.04 LTS + +* Ubuntu 14.04 LTS + +* SuSE Linux Enterprise 11 + +* SuSE Linux Enterprise 12 From 1af73b706148a64a470f79be297791b2586dbdd3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 3 Jul 2015 17:08:32 +0300 Subject: [PATCH 75/97] Fixed common sharding functions referring to schemarouter. --- server/modules/routing/schemarouter/sharding_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/schemarouter/sharding_common.c b/server/modules/routing/schemarouter/sharding_common.c index d69b12d02..cf6bf2c12 100644 --- a/server/modules/routing/schemarouter/sharding_common.c +++ b/server/modules/routing/schemarouter/sharding_common.c @@ -46,7 +46,7 @@ bool extract_database(GWBUF* buf, char* str) tok = strtok_r(query," ;",&saved); if(tok == NULL || strcasecmp(tok,"use") != 0) { - skygw_log_write(LOGFILE_ERROR,"Schemarouter: Malformed chage database packet."); + skygw_log_write(LOGFILE_ERROR,"extract_database: Malformed chage database packet."); succp = false; goto retblock; } @@ -54,7 +54,7 @@ bool extract_database(GWBUF* buf, char* str) tok = strtok_r(NULL," ;",&saved); if(tok == NULL) { - skygw_log_write(LOGFILE_ERROR,"Schemarouter: Malformed chage database packet."); + skygw_log_write(LOGFILE_ERROR,"extract_database: Malformed chage database packet."); succp = false; goto retblock; } From 5bdfa42aa7c961097933bc9961a81f61a39def5e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 06:31:52 +0300 Subject: [PATCH 76/97] Fixed Building from Source document. --- .../Building-MaxScale-from-Source-Code.md | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md index 95236ed7e..2d8f0661e 100644 --- a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md +++ b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md @@ -87,16 +87,13 @@ You will also need some version specific packages. #### Earlier versions of Ubuntu or Debian -For these, you will need to obtain the MariaDB embedded library. It has to be manually extracted from the tarball. But first ascertain what version of glibc is installed. Run the command: +For these, you will need to obtain the MariaDB embedded library. It has to be manually extracted from the tarballs at the MariaDB site. But first ascertain what version of glibc is installed. Run the command: ``` dpkg -l | grep libc6 ``` -which will show the version number. If the version is less than 2.14 you should obtain the library from: -[https://downloads.mariadb.org/interstitial/mariadb-5.5.41/bintar-linux-x86_64/mariadb-5.5.41-linux-x86_64.tar.gz](https://downloads.mariadb.org/interstitial/mariadb-5.5.41/bintar-linux-x86_64/mariadb-5.5.41-linux-x86_64.tar.gz). -Otherwise, from: -[https://downloads.mariadb.org/interstitial/mariadb-5.5.41/bintar-linux-glibc_214-x86_64/mariadb-5.5.41-linux-glibc_214-x86_64.tar.gz](https://downloads.mariadb.org/interstitial/mariadb-5.5.41/bintar-linux-glibc_214-x86_64/mariadb-5.5.41-linux-glibc_214-x86_64.tar.gz) +which will show the version number. For versions older than 2.14 you should obtain the library which supports GLIBC versions older than 2.14 and for newer versions, the library which supports newer GLIBC versions should be used. The suggested location for extracting the tarball is `/usr` so the operation can be done by the following commands: @@ -107,22 +104,6 @@ The suggested location for extracting the tarball is `/usr` so the operation can where /path/to/mariadb.library.tar.gz is replaced by the actual path and name of the downloaded tarball. -## OpenSUSE - -At the time this guide was written, the MariaDB development packages for OpenSUSE were broken and the build failed. - -The packages required are: - -``` -gcc gcc-c++ ncurses-devel bison glibc-devel cmake libgcc_s1 perl -make libtool libopenssl-devel libaio libaio-devel -libedit-devel librabbitmq-devel - MariaDB-devel MariaDB-client MariaDB-server -``` - -If zypper ask which MariaDB client should be installed `MariaDB-client` or `mariadb-client` - please select `MariaDB-client`. This is the package provided by the MariaDB repository. - # Obtaining the MaxScale Source Code Now clone the GitHub project to your machine either via the web interface, your favorite graphical interface or the git command line @@ -152,16 +133,15 @@ wipe the build directory clean without the danger of deleting important files wh something goes wrong. Building 'out-of-source' also allows you to have multiple configurations of MaxScale at the same time. -The default values that CMake uses can be found in the 'macros.cmake' file. -If you wish to change these, edit the 'macros.cmake' file or define the -variables manually at configuration time. +The default values that MaxScale uses for CMake can be found in the 'macros.cmake' file under the `cmake` folder. +If you wish to change these, edit the 'macros.cmake' file or define the variables manually at configuration time. To display all CMake variables with their descriptions: ``` cmake .. -LH ``` -This is a useful command if you have your libraries installed in non-standard locations. +This is a useful command if you have your libraries installed in non-standard locations and need to provide them manually. When you are ready to run cmake, provide the following command: @@ -241,7 +221,7 @@ $ make install This will result in an installation being created which is identical to that which would be achieved by installing the binary package. -By default, MaxScale installs to `/usr/local/mariadb-maxscale` and places init.d scripts and ldconfig files into their folders. Change the `CMAKE_INSTALL_PREFIX` variable to your desired installation directory and set `WITH_SCRIPTS=N` to prevent the init.d script and ldconfig file installation. +When building from source, MaxScale installs to `/usr/local/` and places init.d scripts and ldconfig files into their folders. Change the `CMAKE_INSTALL_PREFIX` variable to your desired installation directory and set `WITH_SCRIPTS=N` to prevent the init.d script and ldconfig file installation. Other useful targets for Make are `documentation`, which generates the Doxygen documentation, and `uninstall` which uninstall MaxScale binaries after an install. From 9d5d6da865d11a5daf391ad0562f0a7472bb2259 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 06:33:49 +0300 Subject: [PATCH 77/97] Removed Fedora references from the source building documentation. --- .../Building-MaxScale-from-Source-Code.md | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md index 2d8f0661e..96147c0f1 100644 --- a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md +++ b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md @@ -22,9 +22,9 @@ After following the instructions on that site you should have a working MariaDB The full list of dependencies for the most common distributions is provided in this section. If your system is not listed here, MaxScale building isn't guaranteed to be compatible but might still be successful. -## RHEL, CentOS and Fedora +## RHEL and CentOS -You will need to install all of the following packages for all versions of RHEL, CentOS and Fedora. +You will need to install all of the following packages for all versions of RHEL and CentOS. ``` gcc gcc-c++ ncurses-devel bison glibc-devel cmake libgcc perl make libtool @@ -39,7 +39,7 @@ rpm-build There are also some version specific packages you need to install. -#### RHEL 6, 7, CentOS 6, 7, Fedora: +#### RHEL 6, 7, CentOS 6, 7: ``` libedit-devel @@ -51,17 +51,11 @@ libedit-devel mariadb-devel mariadb-embedded-devel ``` -#### RHEL 5, 6, CentOS 5, 6, Fedora 19, 20 +#### RHEL 5, 6, CentOS 5, 6 ``` MariaDB-devel MariaDB-server ``` -#### Fedora 19, 20 - -``` -systemtap-sdt-devel -``` - ## Ubuntu and Debian These packages are required on all versions of Ubuntu and Debian. From c2e51f2e983df1bbd47f9253c7106a2041545a8a Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 08:16:29 +0300 Subject: [PATCH 78/97] Fix to MXS-223: https://mariadb.atlassian.net/browse/MXS-223 Added a temporary buffer to prevent losing the pointer to the cloned buffer when dcb->func.write fails. --- .../routing/readwritesplit/readwritesplit.c | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 3c329f646..be2fb68df 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -4011,7 +4011,7 @@ static bool execute_sescmd_in_backend( bool succp; int rc = 0; sescmd_cursor_t* scur; - + GWBUF* buf; if(backend_ref == NULL) { skygw_log_write(LE,"Error: NULL parameter passed to execute_sescmd_in_backend. (%s:%d)",__FILE__,__LINE__); @@ -4048,27 +4048,9 @@ static bool execute_sescmd_in_backend( /** Cursor is left active when function returns. */ sescmd_cursor_set_active(scur, true); } -#if defined(SS_DEBUG) - LOGIF(LT, tracelog_routed_query(scur->scmd_cur_rses, - "execute_sescmd_in_backend", - backend_ref, - sescmd_cursor_clone_querybuf(scur))); - { - GWBUF* tmpbuf = sescmd_cursor_clone_querybuf(scur); - uint8_t* ptr = GWBUF_DATA(tmpbuf); - unsigned char cmd = MYSQL_GET_COMMAND(ptr); - - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [execute_sescmd_in_backend] Just before write, fd " - "%d : cmd %s.", - pthread_self(), - dcb->fd, - STRPACKETTYPE(cmd)))); - gwbuf_free(tmpbuf); - } -#endif /*< SS_DEBUG */ + buf = sescmd_cursor_clone_querybuf(scur); + switch (scur->scmd_cur_cmd->my_sescmd_packet_type) { case MYSQL_COM_CHANGE_USER: /** This makes it possible to handle replies correctly */ @@ -4077,7 +4059,7 @@ static bool execute_sescmd_in_backend( dcb, NULL, dcb->session, - sescmd_cursor_clone_querybuf(scur)); + buf); break; case MYSQL_COM_INIT_DB: @@ -4103,10 +4085,11 @@ static bool execute_sescmd_in_backend( * Mark session command buffer, it triggers writing * MySQL command to protocol */ + gwbuf_set_type(scur->scmd_cur_cmd->my_sescmd_buf, GWBUF_TYPE_SESCMD); rc = dcb->func.write( dcb, - sescmd_cursor_clone_querybuf(scur)); + buf); break; } @@ -4116,6 +4099,7 @@ static bool execute_sescmd_in_backend( } else { + while((buf = GWBUF_CONSUME_ALL(buf)) != NULL); succp = false; } return_succp: From f7e165612f6d20d4b6f790c33b76432ccce28b05 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 14:40:13 +0300 Subject: [PATCH 79/97] Added Upgrading to 1.2 document. --- Documentation/Upgrading-To-MaxScale-1.2.0.md | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/Upgrading-To-MaxScale-1.2.0.md diff --git a/Documentation/Upgrading-To-MaxScale-1.2.0.md b/Documentation/Upgrading-To-MaxScale-1.2.0.md new file mode 100644 index 000000000..a48750076 --- /dev/null +++ b/Documentation/Upgrading-To-MaxScale-1.2.0.md @@ -0,0 +1,24 @@ +# Upgrading MaxScale from 1.1 to 1.2 + +This document describes upgrading MaxScale from version 1.1.1 to 1.2 and the major differences in the new version compared to the old version. The major changes can be found in the `Changelog.txt` file in the installation directory and the official release notes in the `ReleaseNotes.txt` file. + +## Installation + +Before starting the upgrade, we recommend you back up your configuration, log and binary log files in `/usr/local/mariadb-maxscale/`. + +Upgrading MaxScale will copy the `MaxScale.cnf` file in `/usr/local/mariadb-maxscale/etc/` to `/etc/` and renamed to `maxscale.cnf`. Binary log files are not automatically copied and should be manually moved from `/usr/local/mariadb-maxscale` to `/var/lib/maxscale/`. + +## File location changes + +MaxScale 1.2 follows the [FHS-standard](http://www.pathname.com/fhs/) and installs to `/usr/` and `/var/` subfolders. Here are the major changes and file locations. + +* Configuration files are located in `/etc/` and use lowercase letters: `/etc/maxscale.cnf` +* Binary files are in `/usr/bin/` +* Libraries and modules are in `/usr/lib64/maxscale/`. If you are using custom modules, please make sure they are in this directory before starting MaxScale. +* Log files are in the `var/log/maxscale/` folder +* MaxScale's PID file is located in `/var/run/maxscale/maxscale.pid` +* Data files and other persistent files are in `/var/lib/maxscale/` + +## Running MaxScale without root permissions + +MaxScale can run as a non-root user with the 1.2 version. RPM and DEB packages install the `maxscale` user and `maxscale` group which are used by the init scripts and systemd configuration files. If you are installing from a binary tarball, you can run the `postinst` script included in it to manually create these groups. From 309e6881688e20a79d0ed054c461f6b30e7530c6 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 15:03:23 +0300 Subject: [PATCH 80/97] Added link to 1.2 upgrade document in Documentation-Contents.md --- Documentation/Documentation-Contents.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index fc51d83ec..f19797d55 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -19,7 +19,8 @@ ## Upgrading MaxScale -- [Upgrading MaxScale to 1.1.0](Upgrading-To-MaxScale-1.1.0.md) +- [Upgrading MaxScale from 1.1.1 to 1.2.0](Upgrading-To-MaxScale-1.2.0.md) +- [Upgrading MaxScale from 1.0.5 to 1.1.0](Upgrading-To-MaxScale-1.1.0.md) ## Reference From 082a78ac3ded5947f2d0af6ca1a6ad7d04099eb4 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 16:02:29 +0300 Subject: [PATCH 81/97] Updated release notes. --- .../MaxScale-1.2.0-Release-Notes.md | 49 +++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index de4b7a461..c1252a932 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -34,31 +34,30 @@ Now you can configure MaxScale monitor module to automatically launch a script w Here is a list of bugs fixed since the release of MaxScale 1.1.1. -| Key | Summary | -| --- | -------- | -| [MXS-24](https://mariadb.atlassian.net/browse/MXS-24) | bugzillaId-604: Module load path documentation issues ... | -| [MXS-40](https://mariadb.atlassian.net/browse/MXS-40) | Display logged in users | -| [MXS-113](https://mariadb.atlassian.net/browse/MXS-113) | MaxScale seems to fail if built against MariaDB 10.0 libraries | -| [MXS-116](https://mariadb.atlassian.net/browse/MXS-116) | Do not run maxscale as root. | -| [MXS-117](https://mariadb.atlassian.net/browse/MXS-117) | Allow configuration of the log file directory | -| [MXS-125](https://mariadb.atlassian.net/browse/MXS-125) | inconsistency in maxkeys/maxpassword output and parameters | -| [MXS-128](https://mariadb.atlassian.net/browse/MXS-128) | cyclic dependency utils -> log_manager -> utils | -| [MXS-136](https://mariadb.atlassian.net/browse/MXS-136) | Check for MaxScale replication heartbeat table existence before creating | -| [MXS-137](https://mariadb.atlassian.net/browse/MXS-137) | cannot get sql for queries with length >= 0x80 | -| [MXS-139](https://mariadb.atlassian.net/browse/MXS-139) | Schemarouter authentication for wildcard grants fails without optimize_wildcard | -| [MXS-140](https://mariadb.atlassian.net/browse/MXS-140) | strip_db_esc does not work without auth_all_servers | -| [MXS-162](https://mariadb.atlassian.net/browse/MXS-162) | Fix Incorrect info in Configuration Guide | -| [MXS-165](https://mariadb.atlassian.net/browse/MXS-165) | Concurrency issue while incrementing sessions in qlafilter | -| [MXS-166](https://mariadb.atlassian.net/browse/MXS-166) | Memory leak when creating a new event | -| [MXS-171](https://mariadb.atlassian.net/browse/MXS-171) | Allow reads on master for readwritesplit | -| [MXS-176](https://mariadb.atlassian.net/browse/MXS-176) | Missing dependencies in documentation | -| [MXS-180](https://mariadb.atlassian.net/browse/MXS-180) | MariaDB10 binlog router compatibilty | -| [MXS-181](https://mariadb.atlassian.net/browse/MXS-181) | Poor performance on TCP connection due to Nagle's algoritm | -| [MXS-182](https://mariadb.atlassian.net/browse/MXS-182) | SHOW SLAVE STATUS and maxadmin "show services" for binlog router needs updated when used with MariaDB 10 Master | -| [MXS-212](https://mariadb.atlassian.net/browse/MXS-212) | Stopped services accept connections | -| [MXS-225](https://mariadb.atlassian.net/browse/MXS-225) | RPM Debug build packages have no debugging symbols | -| [MXS-227](https://mariadb.atlassian.net/browse/MXS-227) | Memory leak in Galera Monitor | -| [MXS-244](https://mariadb.atlassian.net/browse/MXS-244) | Memory leak when using prepared statements without arguments | + * [MXS-24](https://mariadb.atlassian.net/browse/MXS-24): bugzillaId-604: Module load path documentation issues ... + * [MXS-40](https://mariadb.atlassian.net/browse/MXS-40): Display logged in users + * [MXS-113](https://mariadb.atlassian.net/browse/MXS-113): MaxScale seems to fail if built against MariaDB 10.0 libraries + * [MXS-116](https://mariadb.atlassian.net/browse/MXS-116): Do not run maxscale as root. + * [MXS-117](https://mariadb.atlassian.net/browse/MXS-117): Allow configuration of the log file directory + * [MXS-125](https://mariadb.atlassian.net/browse/MXS-125): inconsistency in maxkeys/maxpassword output and parameters + * [MXS-128](https://mariadb.atlassian.net/browse/MXS-128): cyclic dependency utils -> log_manager -> utils + * [MXS-136](https://mariadb.atlassian.net/browse/MXS-136): Check for MaxScale replication heartbeat table existence before creating + * [MXS-137](https://mariadb.atlassian.net/browse/MXS-137): cannot get sql for queries with length >= 0x80 + * [MXS-139](https://mariadb.atlassian.net/browse/MXS-139): Schemarouter authentication for wildcard grants fails without optimize_wildcard + * [MXS-140](https://mariadb.atlassian.net/browse/MXS-140): strip_db_esc does not work without auth_all_servers + * [MXS-162](https://mariadb.atlassian.net/browse/MXS-162): Fix Incorrect info in Configuration Guide + * [MXS-165](https://mariadb.atlassian.net/browse/MXS-165): Concurrency issue while incrementing sessions in qlafilter + * [MXS-166](https://mariadb.atlassian.net/browse/MXS-166): Memory leak when creating a new event + * [MXS-171](https://mariadb.atlassian.net/browse/MXS-171): Allow reads on master for readwritesplit + * [MXS-176](https://mariadb.atlassian.net/browse/MXS-176): Missing dependencies in documentation + * [MXS-179](https://mariadb.atlassian.net/browse/MXS-179): Keep configuration changes in synch across MaxScale Mate Nodes + * [MXS-180](https://mariadb.atlassian.net/browse/MXS-180): MariaDB10 binlog router compatibilty + * [MXS-181](https://mariadb.atlassian.net/browse/MXS-181): Poor performance on TCP connection due to Nagle's algoritm + * [MXS-182](https://mariadb.atlassian.net/browse/MXS-182): SHOW SLAVE STATUS and maxadmin "show services" for binlog router needs updated when used with MariaDB 10 Master + * [MXS-212](https://mariadb.atlassian.net/browse/MXS-212): Stopped services accept connections + * [MXS-225](https://mariadb.atlassian.net/browse/MXS-225): RPM Debug build packages have no debugging symbols + * [MXS-227](https://mariadb.atlassian.net/browse/MXS-227): Memory leak in Galera Monitor + * [MXS-244](https://mariadb.atlassian.net/browse/MXS-244): Memory leak when using prepared statements without arguments ## Known Issues and Limitations From 72844c9e138ebd6c92785df1ca027f8a21e7af1d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 7 Jul 2015 16:22:26 +0300 Subject: [PATCH 82/97] Added lsyncd guide to release notes. --- Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md index c1252a932..43560ddff 100644 --- a/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-1.2.0-Release-Notes.md @@ -30,6 +30,9 @@ MaxScale now supports SSL/TLS encrypted connections to MaxScale. ### Launchable scripts Now you can configure MaxScale monitor module to automatically launch a script when it detects change in the state of a backend server. The script can be any customer script defined by you to take diagnostic or reporting action. With this you can easily customize MaxScale's behavior. +### Lsyncd configuration guide +A new tutorial has beed added which helps you keep MaxScale's configuration files in sync across multiple hosts. This allows for easier HA setups with MaxScale and guarantees up-to-date configuration files on all nodes. The tutorial can be found [here](../Reference/MaxScale-HA-with-lsyncd.md). + ## Bug fixes Here is a list of bugs fixed since the release of MaxScale 1.1.1. From a72f462e2d0ca7ea8475da8a08e8aa953c8a7945 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 08:48:27 +0100 Subject: [PATCH 83/97] Fixes for MXS-196 and other related problems. --- server/core/dcb.c | 19 +++++- server/core/poll.c | 95 ++++++++++++++++++-------- server/include/dcb.h | 3 +- server/modules/protocol/mysql_client.c | 7 +- 4 files changed, 92 insertions(+), 32 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 8f2f159d4..99a01a640 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -724,7 +724,7 @@ int rc; */ rc = poll_add_dcb(dcb); - if (rc == DCBFD_CLOSED) { + if (rc) { dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); dcb_final_free(dcb); return NULL; @@ -2362,6 +2362,23 @@ bool dcb_set_state( return succp; } +void dcb_revert_state( + DCB* dcb, + const dcb_state_t new_state, + dcb_state_t old_state) +{ + CHK_DCB(dcb); + spinlock_acquire(&dcb->dcb_initlock); + + if ((DCB_STATE_POLLING == new_state || DCB_STATE_LISTENING == new_state) && (DCB_STATE_ALLOC == old_state || DCB_STATE_NOPOLLING == old_state)) + { + dcb->state = old_state; + spinlock_release(&dcb->dcb_initlock); + return; + } + else assert(false); +} + static bool dcb_set_state_nomutex( DCB* dcb, const dcb_state_t new_state, diff --git a/server/core/poll.c b/server/core/poll.c index 9a1a5565d..24b27006f 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -186,6 +186,11 @@ static struct { */ static void poll_loadav(void *); +/** + * Function to analyse error return from epoll_ctl + */ +static int poll_resolve_error(int, bool); + /** * Initialise the polling system we are using for the gateway. * @@ -275,20 +280,9 @@ poll_add_dcb(DCB *dcb) */ if (dcb_set_state(dcb, new_state, &old_state)) { rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); - - if (rc != 0) { - int eno = errno; - errno = 0; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Adding dcb %p in state %s " - "to poll set failed. epoll_ctl failed due " - "%d, %s.", - dcb, - STRDCBSTATE(dcb->state), - eno, - strerror(eno)))); - } else { + if (rc) rc = poll_resolve_error(errno, true); + if (0 == rc) + { LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [poll_add_dcb] Added dcb %p in state %s to " @@ -297,7 +291,7 @@ poll_add_dcb(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - ss_info_dassert(rc == 0, "Unable to add poll"); /*< trap in debug */ + else dcb_revert_state(dcb, new_state, old_state); } else { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -351,17 +345,7 @@ poll_remove_dcb(DCB *dcb) if (dcb->fd > 0) { rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); - - if (rc != 0) { - int eno = errno; - errno = 0; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : epoll_ctl failed due %d, %s.", - eno, - strerror(eno)))); - } - ss_dassert(rc == 0); /*< trap in debug */ + if (rc) rc = poll_resolve_error(errno, false); } } /*< @@ -380,6 +364,63 @@ return_rc: return rc; } +/** + * Check error returns from epoll_ctl. Most result in a crash since they + * are "impossible". Adding when already present is assumed non-fatal. + * Likewise, removing when not present is assumed non-fatal. + * It is assumed that callers to poll routines can handle the failure + * that results from hitting system limit, although an error is written + * here to record the problem. + * + * @param errornum The errno set by epoll_ctl + * @param adding True for adding to poll list, false for removing + * @return -1 on error or 0 for possibly revised return code + */ +static int +poll_resolve_error(int errornum, bool adding) +{ + if (adding) + { + if (EEXIST == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : epoll_ctl could not add, already exists."))); + // Assume another thread added and no serious harm done + return 0; + } + if (ENOSPC == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "The limit imposed by /proc/sys/fs/epoll/max_user_watches was " + "encountered while trying to register (EPOLL_CTL_ADD) a new " + "file descriptor on an epoll instance."))); + /* Failure - assume handled by callers */ + return -1; + } + } + else + { + /* Must be removing */ + if (ENOENT == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : epoll_ctl could not remove, not found."))); + // Assume another thread removed and no serious harm done + return 0; + } + } + /* Common checks for add or remove - crash MaxScale */ + if (EBADF == errornum) assert (!(EBADF == errornum)); + if (EINVAL == errornum) assert (!(EINVAL == errornum)); + if (ENOMEM == errornum) assert (!(ENOMEM == errornum)); + if (EPERM == errornum) assert (!(EPERM == errornum)); + /* Undocumented error number */ + assert(false); +} + #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make * debugging easier. */ @@ -1605,7 +1646,7 @@ RESULT_ROW *row; } /** - * Return a resultset that has the current set of services in it + * Return a result set that has the current set of services in it * * @return A Result set */ diff --git a/server/include/dcb.h b/server/include/dcb.h index 19f1e72ea..7f0997a9a 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -338,7 +338,8 @@ int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, vo int dcb_isvalid(DCB *); /* Check the DCB is in the linked list */ int dcb_count_by_usage(DCB_USAGE); /* Return counts of DCBs */ -bool dcb_set_state(DCB* dcb, dcb_state_t new_state, dcb_state_t* old_state); +bool dcb_set_state(DCB *dcb, dcb_state_t new_state, dcb_state_t *old_state); +void dcb_revert_state(DCB *dcb, const dcb_state_t new_state, dcb_state_t old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index c3e463139..f4d3663a9 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1451,8 +1451,8 @@ int gw_MySQLListener( // add listening socket to poll structure if (poll_add_dcb(listen_dcb) == -1) { fprintf(stderr, - "\n* Failed to start polling the socket due error " - "%i, %s.\n\n", + "\n* MaxScale encountered system limit while " + "attempting to register on an epoll instance.\n\n", errno, strerror(errno)); return 0; @@ -1687,7 +1687,8 @@ int gw_MySQLAccept(DCB *listener) client_dcb, 1, 0, - "MaxScale internal error."); + "MaxScale encountered system limit while " + "attempting to register on an epoll instance."); /** close client_dcb */ dcb_close(client_dcb); From 18a95eeb71892a50c0ddc721138388748aa6ed3f Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 14:50:01 +0100 Subject: [PATCH 84/97] Simplify adding and removing DCBs from polling, improve error handling. Remove dcb_set_state functions as not adding value. --- server/core/dcb.c | 200 ++----------------------------------------- server/core/poll.c | 164 +++++++++++++++++++++-------------- server/include/dcb.h | 2 - 3 files changed, 105 insertions(+), 261 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 99a01a640..abdc4dde6 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -84,10 +84,6 @@ static SPINLOCK dcbspin = SPINLOCK_INIT; static SPINLOCK zombiespin = SPINLOCK_INIT; static void dcb_final_free(DCB *dcb); -static bool dcb_set_state_nomutex( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state); static void dcb_call_callback(DCB *dcb, DCB_REASON reason); static DCB* dcb_get_next (DCB* dcb); static int dcb_null_write(DCB *dcb, GWBUF *buf); @@ -291,8 +287,7 @@ dcb_add_to_zombieslist(DCB *dcb) * Set state which indicates that it has been added to zombies * list. */ - succp = dcb_set_state(dcb, DCB_STATE_ZOMBIE, &prev_state); - ss_info_dassert(succp, "Failed to set DCB_STATE_ZOMBIE"); + dcb->state = DCB_STATE_ZOMBIE; spinlock_release(&zombiespin); } @@ -604,7 +599,7 @@ bool succp = false; &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - succp = dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; ss_dassert(succp); dcb_next = dcb->memdata.next; dcb_final_free(dcb); @@ -644,7 +639,7 @@ int rc; if ((funcs = (GWPROTOCOL *)load_module(protocol, MODULE_PROTOCOL)) == NULL) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -682,7 +677,7 @@ int rc; dcb, session->client, session->client->fd))); - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); return NULL; } else { @@ -725,7 +720,7 @@ int rc; rc = poll_add_dcb(dcb); if (rc) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); return NULL; } @@ -1920,7 +1915,7 @@ dcb_drain_writeq_SSL(DCB *dcb) } /** - * Removes dcb from poll set, and adds it to zombies list. As a consequense, + * Removes dcb from poll set, and adds it to zombies list. As a consequence, * 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. @@ -1947,7 +1942,6 @@ dcb_close(DCB *dcb) */ if (dcb->state == DCB_STATE_ALLOC) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); dcb_final_free(dcb); return; } @@ -2341,188 +2335,6 @@ void dcb_hashtable_stats( dcb_printf(dcb, "\tLongest chain length: %d\n", longest); } - -bool dcb_set_state( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state) -{ - bool succp; - dcb_state_t state ; - - CHK_DCB(dcb); - spinlock_acquire(&dcb->dcb_initlock); - succp = dcb_set_state_nomutex(dcb, new_state, &state); - - spinlock_release(&dcb->dcb_initlock); - - if (old_state != NULL) { - *old_state = state; - } - return succp; -} - -void dcb_revert_state( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t old_state) -{ - CHK_DCB(dcb); - spinlock_acquire(&dcb->dcb_initlock); - - if ((DCB_STATE_POLLING == new_state || DCB_STATE_LISTENING == new_state) && (DCB_STATE_ALLOC == old_state || DCB_STATE_NOPOLLING == old_state)) - { - dcb->state = old_state; - spinlock_release(&dcb->dcb_initlock); - return; - } - else assert(false); -} - -static bool dcb_set_state_nomutex( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state) -{ - bool succp = false; - dcb_state_t state = DCB_STATE_UNDEFINED; - - CHK_DCB(dcb); - - state = dcb->state; - - if (old_state != NULL) { - *old_state = state; - } - - switch (state) { - case DCB_STATE_UNDEFINED: - dcb->state = new_state; - succp = true; - break; - - case DCB_STATE_ALLOC: - switch (new_state) { - /** fall through, for client requests */ - case DCB_STATE_POLLING: - /** fall through, for connect listeners */ - case DCB_STATE_LISTENING: - /** for failed connections */ - case DCB_STATE_DISCONNECTED: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_POLLING: - switch(new_state) { - case DCB_STATE_NOPOLLING: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_LISTENING: - switch(new_state) { - case DCB_STATE_NOPOLLING: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_NOPOLLING: - switch (new_state) { - /** Stopped services which are restarting will go from - * DCB_STATE_NOPOLLING to DCB_STATE_LISTENING.*/ - case DCB_STATE_LISTENING: - case DCB_STATE_ZOMBIE: /*< fall through */ - dcb->state = new_state; - case DCB_STATE_POLLING: /*< ok to try but state can't change */ - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_ZOMBIE: - switch (new_state) { - case DCB_STATE_DISCONNECTED: /*< fall through */ - dcb->state = new_state; - case DCB_STATE_POLLING: /*< ok to try but state can't change */ - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_DISCONNECTED: - switch (new_state) { - case DCB_STATE_FREED: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_FREED: - ss_dassert(old_state != NULL); - break; - - default: - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unknown dcb state %s for " - "dcb %p", - STRDCBSTATE(dcb->state), - dcb))); - ss_dassert(false); - break; - } /*< switch (dcb->state) */ - - if (succp) { - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [dcb_set_state_nomutex] dcb %p fd %d %s -> %s", - pthread_self(), - dcb, - dcb->fd, - STRDCBSTATE(state), - STRDCBSTATE(dcb->state)))); - } - else - { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_set_state_nomutex] Failed " - "to change state of DCB %p. " - "Old state %s > new state %s.", - pthread_self(), - dcb, - (old_state == NULL ? "NULL" : STRDCBSTATE(*old_state)), - STRDCBSTATE(new_state)))); - } - return succp; -} - /** * Write data to a socket through an SSL structure. The SSL structure is linked to a DCB's socket * and all communication is encrypted and done via the SSL structure. diff --git a/server/core/poll.c b/server/core/poll.c index 24b27006f..9eed91bba 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -189,7 +189,7 @@ static void poll_loadav(void *); /** * Function to analyse error return from epoll_ctl */ -static int poll_resolve_error(int, bool); +static int poll_resolve_error(DCB *, int, bool); /** * Initialise the polling system we are using for the gateway. @@ -252,7 +252,7 @@ int poll_add_dcb(DCB *dcb) { int rc = -1; - dcb_state_t old_state = DCB_STATE_UNDEFINED; + dcb_state_t old_state = dcb->state; dcb_state_t new_state; struct epoll_event ev; @@ -268,47 +268,71 @@ poll_add_dcb(DCB *dcb) /*< * Choose new state according to the role of dcb. */ + spinlock_acquire(&dcb->dcb_initlock); if (dcb->dcb_role == DCB_ROLE_REQUEST_HANDLER) { new_state = DCB_STATE_POLLING; } else { ss_dassert(dcb->dcb_role == DCB_ROLE_SERVICE_LISTENER); new_state = DCB_STATE_LISTENING; } + /* + * Check DCB current state seems sensible + */ + if (DCB_STATE_DISCONNECTED == dcb->state + || DCB_STATE_ZOMBIE == dcb->state + || DCB_STATE_UNDEFINED == dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_add_dcb] Error : existing state of dcb %p " + "is %s, but this should be impossible, crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + assert(false); + } + if (DCB_STATE_POLLING == dcb->state + || DCB_STATE_LISTENING == dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_add_dcb] Error : existing state of dcb %p " + "is %s, but this is probably an error, not crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + } /*< * If dcb is in unexpected state, state change fails indicating that dcb * is not polling anymore. */ - if (dcb_set_state(dcb, new_state, &old_state)) { - rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); - if (rc) rc = poll_resolve_error(errno, true); - if (0 == rc) - { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [poll_add_dcb] Added dcb %p in state %s to " - "poll set.", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); - } - else dcb_revert_state(dcb, new_state, old_state); - } else { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unable to set new state for dcb %p " - "in state %s. Adding to poll set failed.", - dcb, - STRDCBSTATE(dcb->state)))); + dcb->state = new_state; + spinlock_release(&dcb->dcb_initlock); + /* + * The only possible failure that will not cause a crash is + * running out of system resources. + */ + rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); + if (rc) + { + rc = poll_resolve_error(dcb, errno, true); } - + if (0 == rc) + { + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [poll_add_dcb] Added dcb %p in state %s to poll set.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + } + else dcb->state = old_state; return rc; } /** * Remove a descriptor from the set of descriptors within the * polling environment. - * The state change command may fail because concurrent threads may call - * dcb_set_state simultaneously and the conflict is prevented in dcb_set_state. * * @param dcb The descriptor to remove * @return -1 on error or 0 on success @@ -316,52 +340,53 @@ poll_add_dcb(DCB *dcb) int poll_remove_dcb(DCB *dcb) { - struct epoll_event ev; int rc = -1; - dcb_state_t old_state = DCB_STATE_UNDEFINED; - dcb_state_t new_state = DCB_STATE_NOPOLLING; CHK_DCB(dcb); + spinlock_acquire(&dcb->dcb_initlock); /*< It is possible that dcb has already been removed from the set */ - if (dcb->state != DCB_STATE_POLLING && - dcb->state != DCB_STATE_LISTENING) - { - if (dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE) - { - rc = 0; - } - goto return_rc; + if (dcb->state == DCB_STATE_NOPOLLING || + dcb->state == DCB_STATE_ZOMBIE) + { + spinlock_release(&dcb->dcb_initlock); + return 0; + } + if (DCB_STATE_POLLING != dcb->state + && DCB_STATE_LISTENING != dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_remove_dcb] Error : existing state of dcb %p " + "is %s, but this is probably an error, not crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); } /*< * Set state to NOPOLLING and remove dcb from poll set. */ - if (dcb_set_state(dcb, new_state, &old_state)) - { - /** - * Only positive fds can be removed from epoll set. - */ - if (dcb->fd > 0) - { - rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); - if (rc) rc = poll_resolve_error(errno, false); - } - } - /*< - * This call was redundant, but the end result is correct. + dcb->state = DCB_STATE_NOPOLLING; + /** + * Only positive fds can be removed from epoll set. + * But this test was removed by Martin as it is hard to + * see that there should ever be a situation where + * fd isn't positive and the DCB is also in the poll list. + */ + /* if (dcb->fd > 0) { */ + spinlock_release(&dcb->dcb_initlock); + rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); + /** + * The poll_resolve_error function will always + * return 0 or crash. So if it returns non-zero result, + * things have gone wrong and we crash. */ - else if (old_state == new_state) - { - rc = 0; - goto return_rc; - } - + if (rc) rc = poll_resolve_error(dcb, errno, false); + if (rc) assert(false); /*< Set bit for each maxscale thread */ - bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); - rc = 0; -return_rc: + bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); return rc; + /* } */ } /** @@ -377,7 +402,7 @@ return_rc: * @return -1 on error or 0 for possibly revised return code */ static int -poll_resolve_error(int errornum, bool adding) +poll_resolve_error(DCB *dcb, int errornum, bool adding) { if (adding) { @@ -385,7 +410,10 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : epoll_ctl could not add, already exists."))); + "%lu [poll_resolve_error] Error : epoll_ctl could not add, " + "already exists for DCB %p.", + pthread_self(), + dcb))); // Assume another thread added and no serious harm done return 0; } @@ -393,9 +421,12 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "The limit imposed by /proc/sys/fs/epoll/max_user_watches was " + "%lu [poll_resolve_error] The limit imposed by " + "/proc/sys/fs/epoll/max_user_watches was " "encountered while trying to register (EPOLL_CTL_ADD) a new " - "file descriptor on an epoll instance."))); + "file descriptor on an epoll instance for dcb %p.", + pthread_self(), + dcb))); /* Failure - assume handled by callers */ return -1; } @@ -407,7 +438,10 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : epoll_ctl could not remove, not found."))); + "%lu [poll_resolve_error] Error : epoll_ctl could not remove, " + "not found, for dcb %p.", + pthread_self(), + dcb))); // Assume another thread removed and no serious harm done return 0; } diff --git a/server/include/dcb.h b/server/include/dcb.h index 7f0997a9a..e33669b72 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -338,8 +338,6 @@ int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, vo int dcb_isvalid(DCB *); /* Check the DCB is in the linked list */ int dcb_count_by_usage(DCB_USAGE); /* Return counts of DCBs */ -bool dcb_set_state(DCB *dcb, dcb_state_t new_state, dcb_state_t *old_state); -void dcb_revert_state(DCB *dcb, const dcb_state_t new_state, dcb_state_t old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); From 77d374cd524559daaf7450e82e258d7ad8fdf37e Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 15:06:45 +0100 Subject: [PATCH 85/97] Fix mistake. --- server/core/poll.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/poll.c b/server/core/poll.c index 9eed91bba..21ad914ae 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -341,7 +341,7 @@ int poll_remove_dcb(DCB *dcb) { int rc = -1; - + struct epoll_event ev; CHK_DCB(dcb); spinlock_acquire(&dcb->dcb_initlock); From 44fcb9bc310e5111becea4e2ccf4818e5be724aa Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 15:29:58 +0100 Subject: [PATCH 86/97] Fix for incorrect password handling. --- server/modules/protocol/mysql_common.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 9b138360a..fcf254d64 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -46,6 +46,9 @@ #include #include +/* The following can be compared using memcmp to detect a null password */ +uint8_t null_client_sha1[MYSQL_SCRAMBLE_LEN]=""; + /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; @@ -577,7 +580,7 @@ int gw_send_authentication_to_backend( if (strlen(dbname)) curr_db = dbname; - if (strlen((char *)passwd)) + if (memcmp(passwd, null_client_sha1, MYSQL_SCRAMBLE_LEN)) curr_passwd = passwd; dcb = conn->owner_dcb; @@ -1122,7 +1125,7 @@ GWBUF* gw_create_change_user_packet( curr_db = db; } - if (strlen((char *)pwd) > 0) + if (memcmp(pwd, null_client_sha1, MYSQL_SCRAMBLE_LEN)) { curr_passwd = pwd; } @@ -1358,12 +1361,7 @@ int gw_check_mysql_scramble_data(DCB *dcb, uint8_t *token, unsigned int token_le gw_bin2hex(hex_double_sha1, password, SHA_DIGEST_LENGTH); } else { /* check if the password is not set in the user table */ - if (!strlen((char *)password)) { - /* Username without password */ - return 0; - } else { - return 1; - } + return memcmp(password, null_client_sha1, MYSQL_SCRAMBLE_LEN) ? 1 : 0; } /*< From fffd8fb73a99e24edc3b8b935f878110965ef38a Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 16:48:53 +0100 Subject: [PATCH 87/97] Unify DCB close processing to single function dcb_close. Remove dcb_add_to_zombieslist (incorporating logic into dcb_close). Alter logic so that DCB that is just allocated will still go to zombie list if dcb->fd is not closed. --- server/core/dcb.c | 186 ++++++++++++---------------- server/core/poll.c | 4 - server/core/test/testdcb.c | 2 +- server/include/dcb.h | 1 - server/modules/protocol/maxscaled.c | 9 +- server/modules/protocol/telnetd.c | 6 +- 6 files changed, 88 insertions(+), 120 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index abdc4dde6..ef259ca4b 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -248,50 +248,6 @@ dcb_free(DCB *dcb) } } -/** - * Add the DCB to the end of zombies list. - * - * Adding to list occurs once per DCB. This is ensured by changing the - * state of DCB to DCB_STATE_ZOMBIE after addition. Prior insertion, DCB state - * is checked and operation proceeds only if state differs from DCB_STATE_ZOMBIE. - * @param dcb The DCB to add to the zombie list - * @return none - */ -void -dcb_add_to_zombieslist(DCB *dcb) -{ - bool succp = false; - dcb_state_t prev_state = DCB_STATE_UNDEFINED; - - CHK_DCB(dcb); - - /*< - * Protect zombies list access. - */ - spinlock_acquire(&zombiespin); - /*< - * If dcb is already added to zombies list, return. - */ - if (dcb->state != DCB_STATE_NOPOLLING) { - ss_dassert(dcb->state != DCB_STATE_POLLING && - dcb->state != DCB_STATE_LISTENING); - spinlock_release(&zombiespin); - return; - } - /*< - * Add closing dcb to the top of the list. - */ - dcb->memdata.next = zombies; - zombies = dcb; - /*< - * Set state which indicates that it has been added to zombies - * list. - */ - dcb->state = DCB_STATE_ZOMBIE; - - spinlock_release(&zombiespin); -} - /* * Clone a DCB for internal use, mostly used for specialist filters * to create dummy clients based on real clients. @@ -600,7 +556,6 @@ bool succp = false; &tls_log_info.li_enabled_logs))); dcb->state = DCB_STATE_DISCONNECTED; - ss_dassert(succp); dcb_next = dcb->memdata.next; dcb_final_free(dcb); dcb = dcb_next; @@ -1928,72 +1883,91 @@ dcb_drain_writeq_SSL(DCB *dcb) void dcb_close(DCB *dcb) { - int rc = 0; + int rc = 0; - CHK_DCB(dcb); + CHK_DCB(dcb); - LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, - "%lu [dcb_close]", - pthread_self()))); - - /** - * dcb_close may be called for freshly created dcb, in which case - * it only needs to be freed. - */ - if (dcb->state == DCB_STATE_ALLOC) + LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, + "%lu [dcb_close]", + pthread_self()))); + + if (DCB_STATE_UNDEFINED == dcb->state + || DCB_STATE_DISCONNECTED == dcb->state + || DCB_STATE_ZOMBIE == dcb->state) + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is not legal for a call to dcb_close. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + assert(false); + } + + /** + * dcb_close may be called for freshly created dcb, in which case + * it only needs to be freed. + */ + if (dcb->state == DCB_STATE_ALLOC && dcb->fd != DCBFD_CLOSED) + { + dcb_final_free(dcb); + return; + } + + /*< + * Stop dcb's listening and modify state accordingly. + */ + if (dcb->state == DCB_STATE_POLLING || dcb->state == DCB_STATE_LISTENING) + { + if (dcb->state == DCB_STATE_LISTENING) { - dcb_final_free(dcb); - return; + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is not expected for a call to dcb_close, although it" + "should be processed correctly. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); } + rc = poll_remove_dcb(dcb); + /* + * Return will always be 0 or function will have crashed + */ + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_close] Removed dcb %p in state %s from " + "poll set.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + /** + * close protocol and router session + */ + if (dcb->func.close != NULL) + { + dcb->func.close(dcb); + } + /** Call possible callback for this DCB in case of close */ + dcb_call_callback(dcb, DCB_REASON_CLOSE); + } + assert (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + + spinlock_acquire(&zombiespin); + + /*< + * Add closing dcb to the top of the list. + */ + dcb->memdata.next = zombies; + zombies = dcb; + /*< + * Set state which indicates that it has been added to zombies + * list. + */ + dcb->state = DCB_STATE_ZOMBIE; - ss_dassert(dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE); - - /*< - * Stop dcb's listening and modify state accordingly. - */ - if (dcb->state == DCB_STATE_POLLING) - { - rc = poll_remove_dcb(dcb); - - if (rc == 0) { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_close] Removed dcb %p in state %s from " - "poll set.", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); - } else { - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "Error : Removing DCB fd == %d in state %s from " - "poll set failed.", - dcb->fd, - STRDCBSTATE(dcb->state)))); - } - - if (rc == 0) - { - /** - * close protocol and router session - */ - if (dcb->func.close != NULL) - { - dcb->func.close(dcb); - } - /** Call possible callback for this DCB in case of close */ - dcb_call_callback(dcb, DCB_REASON_CLOSE); - - if (dcb->state == DCB_STATE_NOPOLLING) - { - dcb_add_to_zombieslist(dcb); - } - } - ss_dassert(dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE); - } + spinlock_release(&zombiespin); } /** diff --git a/server/core/poll.c b/server/core/poll.c index 21ad914ae..0406140a4 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -302,10 +302,6 @@ poll_add_dcb(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - /*< - * If dcb is in unexpected state, state change fails indicating that dcb - * is not polling anymore. - */ dcb->state = new_state; spinlock_release(&dcb->dcb_initlock); /* diff --git a/server/core/test/testdcb.c b/server/core/test/testdcb.c index 2703763f0..13462fc87 100644 --- a/server/core/test/testdcb.c +++ b/server/core/test/testdcb.c @@ -64,7 +64,7 @@ int buflen; ss_info_dassert(!dcb_isvalid(dcb), "Freed DCB must not be valid"); ss_dfprintf(stderr, "\t..done\nMake clone DCB a zombie"); clone->state = DCB_STATE_NOPOLLING; - dcb_add_to_zombieslist(clone); + dcb_close(clone); ss_info_dassert(dcb_get_zombies() == clone, "Clone DCB must be start of zombie list now"); ss_dfprintf(stderr, "\t..done\nProcess the zombies list"); dcb_process_zombies(0); diff --git a/server/include/dcb.h b/server/include/dcb.h index e33669b72..0e5cddb48 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -330,7 +330,6 @@ const char *gw_dcb_state2string(int); /* DCB state to string */ void dcb_printf(DCB *, const char *, ...); /* DCB version of printf */ int dcb_isclient(DCB *); /* the DCB is the client of the session */ 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, void *), diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index 67d51f9bd..0812ea964 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -270,9 +270,7 @@ int n_connect = 0; { atomic_add(&dcb->stats.n_accepts, 1); client_dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); - if (client_dcb == NULL) - { close(so); return n_connect; @@ -283,7 +281,8 @@ int n_connect = 0; if ((maxscaled_pr = (MAXSCALED *)malloc(sizeof(MAXSCALED))) == NULL) { client_dcb->protocol = NULL; - dcb_add_to_zombieslist(client_dcb); + close(so); + dcb_close(client_dcb); return n_connect; } maxscaled_pr->username = NULL; @@ -293,9 +292,9 @@ int n_connect = 0; client_dcb->session = session_alloc(dcb->session->service, client_dcb); - if (poll_add_dcb(client_dcb) == -1) + if (poll_add_dcb(client_dcb)) { - dcb_add_to_zombieslist(dcb); + dcb_close(dcb); return n_connect; } n_connect++; diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index ab5c95b47..9a88307d2 100644 --- a/server/modules/protocol/telnetd.c +++ b/server/modules/protocol/telnetd.c @@ -315,13 +315,13 @@ int n_connect = 0; if (telnetd_pr == NULL) { - dcb_add_to_zombieslist(client_dcb); + dcb_close(client_dcb); return n_connect; } - if (poll_add_dcb(client_dcb) == -1) + if (poll_add_dcb(client_dcb)) { - dcb_add_to_zombieslist(dcb); + dcb_close(dcb); return n_connect; } n_connect++; From 96619e2f8f03994cabf88b5241944a49b9d29b82 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 17:39:32 +0100 Subject: [PATCH 88/97] Allow zombies to be submitted to dcb_close - but why does this happen? --- server/core/dcb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/core/dcb.c b/server/core/dcb.c index ef259ca4b..fc4c412ea 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1891,6 +1891,18 @@ dcb_close(DCB *dcb) "%lu [dcb_close]", pthread_self()))); + if (DCB_STATE_ZOMBIE == dcb->state) + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is unexpected for a call to dcb_close, but not crashing. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + return 0; + } + if (DCB_STATE_UNDEFINED == dcb->state || DCB_STATE_DISCONNECTED == dcb->state || DCB_STATE_ZOMBIE == dcb->state) From 9ee8d11808c6349adb14f7799035f9398f5e1b61 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 17:57:03 +0100 Subject: [PATCH 89/97] Allow for DCB becoming a zombie during processing. --- server/core/dcb.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index fc4c412ea..7a5314580 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1883,8 +1883,6 @@ dcb_drain_writeq_SSL(DCB *dcb) void dcb_close(DCB *dcb) { - int rc = 0; - CHK_DCB(dcb); LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, @@ -1900,7 +1898,7 @@ dcb_close(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - return 0; + return; } if (DCB_STATE_UNDEFINED == dcb->state @@ -1943,9 +1941,10 @@ dcb_close(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - rc = poll_remove_dcb(dcb); + poll_remove_dcb(dcb); /* - * Return will always be 0 or function will have crashed + * Return will always be 0 or function will have crashed, so we + * threw away return value. */ LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, @@ -1964,22 +1963,23 @@ dcb_close(DCB *dcb) /** Call possible callback for this DCB in case of close */ dcb_call_callback(dcb, DCB_REASON_CLOSE); } - assert (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + { + spinlock_acquire(&zombiespin); - spinlock_acquire(&zombiespin); - - /*< - * Add closing dcb to the top of the list. - */ - dcb->memdata.next = zombies; - zombies = dcb; - /*< - * Set state which indicates that it has been added to zombies - * list. - */ - dcb->state = DCB_STATE_ZOMBIE; + /*< + * Add closing dcb to the top of the list. + */ + dcb->memdata.next = zombies; + zombies = dcb; + /*< + * Set state which indicates that it has been added to zombies + * list. + */ + dcb->state = DCB_STATE_ZOMBIE; - spinlock_release(&zombiespin); + spinlock_release(&zombiespin); + } } /** From 5577ef94e9a32fa813291b8780bebcbfe678ba5d Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 08:37:29 +0100 Subject: [PATCH 90/97] Wrap spinlock around more logic; simplify process zombies list logic. --- server/core/dcb.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7a5314580..f350a8ad3 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -484,14 +484,13 @@ bool succp = false; /*< * Move dcb to linked list of victim dcbs. */ + ptr->memdata.next = NULL; if (dcb_list == NULL) { dcb_list = ptr; - dcb = dcb_list; } else { dcb->memdata.next = ptr; - dcb = dcb->memdata.next; } - dcb->memdata.next = NULL; + dcb = ptr; ptr = tptr; } else @@ -1963,10 +1962,10 @@ dcb_close(DCB *dcb) /** Call possible callback for this DCB in case of close */ dcb_call_callback(dcb, DCB_REASON_CLOSE); } + + spinlock_acquire(&zombiespin); if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); { - spinlock_acquire(&zombiespin); - /*< * Add closing dcb to the top of the list. */ @@ -1977,9 +1976,8 @@ dcb_close(DCB *dcb) * list. */ dcb->state = DCB_STATE_ZOMBIE; - - spinlock_release(&zombiespin); } + spinlock_release(&zombiespin); } /** From d4eff72d8a659f33825bc1163092d58267b5d932 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 08:59:52 +0100 Subject: [PATCH 91/97] Fix stupid bug. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index f350a8ad3..210e6d82d 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1964,7 +1964,7 @@ dcb_close(DCB *dcb) } spinlock_acquire(&zombiespin); - if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC) { /*< * Add closing dcb to the top of the list. From 462c8e42ef787499eb37ea671fc21a24eaba4437 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 09:12:38 +0100 Subject: [PATCH 92/97] Fix more subtle bug and expand debug message for dcb_close entry. --- server/core/dcb.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 210e6d82d..87542b5c1 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1885,24 +1885,18 @@ dcb_close(DCB *dcb) CHK_DCB(dcb); LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, - "%lu [dcb_close]", - pthread_self()))); + "%lu [dcb_close] DCB %p in state %s", + pthread_self(), + dcb, + dcb ? STRDCBSTATE(dcb->state) : "Invalid DCB"))); if (DCB_STATE_ZOMBIE == dcb->state) { - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "%lu [dcb_close] Error : Removing DCB %p but was in state %s " - "which is unexpected for a call to dcb_close, but not crashing. ", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); return; } - if (DCB_STATE_UNDEFINED == dcb->state - || DCB_STATE_DISCONNECTED == dcb->state - || DCB_STATE_ZOMBIE == dcb->state) + if (DCB_STATE_UNDEFINED == dcb->state + || DCB_STATE_DISCONNECTED == dcb->state) { LOGIF(LE, (skygw_log_write( LOGFILE_ERROR, From be789855eeb294d2c0b1c8a92505212931c5d935 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 09:43:04 +0100 Subject: [PATCH 93/97] Add lines to revision history. --- server/core/dcb.c | 3 +++ server/core/poll.c | 1 + server/modules/protocol/httpd.c | 1 + 3 files changed, 5 insertions(+) diff --git a/server/core/dcb.c b/server/core/dcb.c index 87542b5c1..d1eb69a5a 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -50,6 +50,9 @@ * 07/05/2014 Mark Riddoch Addition of callback mechanism * 20/06/2014 Mark Riddoch Addition of dcb_clone * 29/05/2015 Markus Makela Addition of dcb_write_SSL + * 07/07/2015 Martin Brampton Merged add to zombieslist into dcb_close, + * fixes for various error situations, + * remove dcb_set_state etc, simplifications. * * @endverbatim */ diff --git a/server/core/poll.c b/server/core/poll.c index 0406140a4..95ae7dc73 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -71,6 +71,7 @@ int max_poll_sleep; * in the loop after the epoll_wait. This allows for better * thread utilisaiton and fairer scheduling of the event * processing. + * 07/07/15 Martin Brampton Simplified add and remove DCB, improve error handling. * * @endverbatim */ diff --git a/server/modules/protocol/httpd.c b/server/modules/protocol/httpd.c index 0de934dec..edd6c75b8 100644 --- a/server/modules/protocol/httpd.c +++ b/server/modules/protocol/httpd.c @@ -33,6 +33,7 @@ * Date Who Description * 08/07/2013 Massimiliano Pinto Initial version * 09/07/2013 Massimiliano Pinto Added /show?dcb|session for all dcbs|sessions + * 07/07/2015 Martin Brampton Fixed problems with DCBs on errors * * @endverbatim */ From 4c8aa02c31b92259fd3f0eba4bf8063f07a0263a Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 16:51:22 +0100 Subject: [PATCH 94/97] Finalise comments; change abort from assert(false) to raise(SIGABRT). --- server/core/dcb.c | 3 ++- server/core/poll.c | 8 +++++--- server/modules/protocol/httpd.c | 1 - server/modules/protocol/maxscaled.c | 1 + server/modules/protocol/mysql_common.c | 1 + server/modules/protocol/telnetd.c | 1 + 6 files changed, 10 insertions(+), 5 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index d1eb69a5a..78f4a821f 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -60,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -1908,7 +1909,7 @@ dcb_close(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - assert(false); + raise(SIGABRT); } /** diff --git a/server/core/poll.c b/server/core/poll.c index 95ae7dc73..a7afa9345 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -15,10 +15,12 @@ * * Copyright MariaDB Corporation Ab 2013-2014 */ + #include #include #include #include +#include #include #include #include @@ -290,7 +292,7 @@ poll_add_dcb(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - assert(false); + raise(SIGABRT); } if (DCB_STATE_POLLING == dcb->state || DCB_STATE_LISTENING == dcb->state) @@ -379,7 +381,7 @@ poll_remove_dcb(DCB *dcb) * things have gone wrong and we crash. */ if (rc) rc = poll_resolve_error(dcb, errno, false); - if (rc) assert(false); + if (rc) raise(SIGABRT); /*< Set bit for each maxscale thread */ bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); return rc; @@ -449,7 +451,7 @@ poll_resolve_error(DCB *dcb, int errornum, bool adding) if (ENOMEM == errornum) assert (!(ENOMEM == errornum)); if (EPERM == errornum) assert (!(EPERM == errornum)); /* Undocumented error number */ - assert(false); + raise(SIGABRT); } #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make diff --git a/server/modules/protocol/httpd.c b/server/modules/protocol/httpd.c index edd6c75b8..0de934dec 100644 --- a/server/modules/protocol/httpd.c +++ b/server/modules/protocol/httpd.c @@ -33,7 +33,6 @@ * Date Who Description * 08/07/2013 Massimiliano Pinto Initial version * 09/07/2013 Massimiliano Pinto Added /show?dcb|session for all dcbs|sessions - * 07/07/2015 Martin Brampton Fixed problems with DCBs on errors * * @endverbatim */ diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index 0812ea964..159718df0 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -58,6 +58,7 @@ extern __thread log_info_t tls_log_info; * Revision History * Date Who Description * 13/06/2014 Mark Riddoch Initial implementation + * 07/07/15 Martin Brampton Correct failure handling * * @endverbatim */ diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index fcf254d64..ea3227699 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -36,6 +36,7 @@ * 03/10/2014 Massimiliano Pinto Added netmask for wildcard in IPv4 hosts. * 24/10/2014 Massimiliano Pinto Added Mysql user@host @db authentication support * 10/11/2014 Massimiliano Pinto Charset at connect is passed to backend during authentication + * 07/07/15 Martin Brampton Fix problem recognising null password * */ diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index 9a88307d2..006fabe99 100644 --- a/server/modules/protocol/telnetd.c +++ b/server/modules/protocol/telnetd.c @@ -67,6 +67,7 @@ extern __thread log_info_t tls_log_info; * Date Who Description * 17/06/2013 Mark Riddoch Initial version * 17/07/2013 Mark Riddoch Addition of login phase + * 07/07/2015 Martin Brampton Call unified dcb_close on error * * @endverbatim */ From b73aedd0ca9e36474ac6407aeff6fc4ef27b7cb0 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 8 Jul 2015 10:05:15 +0300 Subject: [PATCH 95/97] Fixed debug log not working. --- log_manager/log_manager.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index a87157be7..79c2ace30 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -1457,7 +1457,7 @@ int skygw_log_write( * Write log string to buffer and add to file write list. */ - for (i = LOGFILE_FIRST; i Date: Wed, 8 Jul 2015 08:52:07 +0100 Subject: [PATCH 96/97] Change poll control failures to use SIGABRT rather than assert(false). --- server/core/poll.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/core/poll.c b/server/core/poll.c index a7afa9345..4123e0294 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -446,12 +446,14 @@ poll_resolve_error(DCB *dcb, int errornum, bool adding) } } /* Common checks for add or remove - crash MaxScale */ - if (EBADF == errornum) assert (!(EBADF == errornum)); - if (EINVAL == errornum) assert (!(EINVAL == errornum)); - if (ENOMEM == errornum) assert (!(ENOMEM == errornum)); - if (EPERM == errornum) assert (!(EPERM == errornum)); + if (EBADF == errornum) raise(SIGABRT); + if (EINVAL == errornum) raise(SIGABRT); + if (ENOMEM == errornum) raise(SIGABRT); + if (EPERM == errornum) raise(SIGABRT); /* Undocumented error number */ raise(SIGABRT); + /* The following statement should never be reached, but avoids compiler warning */ + return -1; } #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make From 10d690273d953d07144f7b50418a4fd80e74ad07 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Wed, 8 Jul 2015 11:35:41 +0100 Subject: [PATCH 97/97] Resolve compiler warnings --- server/core/adminusers.c | 1 - server/core/config.c | 2 +- server/modules/include/shardrouter.h | 2 ++ server/modules/routing/readwritesplit/readwritesplit.c | 1 - server/modules/routing/schemarouter/shardrouter.c | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/core/adminusers.c b/server/core/adminusers.c index 53734e9cc..e4320ec57 100644 --- a/server/core/adminusers.c +++ b/server/core/adminusers.c @@ -19,7 +19,6 @@ #include #include #include -#define _XOPEN_SOURCE #include #include #include diff --git a/server/core/config.c b/server/core/config.c index 58dd42b7a..60f68a1e1 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -310,7 +310,7 @@ if((monitorhash = hashtable_alloc(5,simple_str_hash,strcmp)) == NULL) return 0; } -hashtable_memory_fns(monitorhash,strdup,NULL,free,NULL); +hashtable_memory_fns(monitorhash,(HASHMEMORYFN)strdup,NULL,(HASHMEMORYFN)free,NULL); /** * Process the data and create the services and servers defined * in the data. diff --git a/server/modules/include/shardrouter.h b/server/modules/include/shardrouter.h index dfd83d520..63572671e 100644 --- a/server/modules/include/shardrouter.h +++ b/server/modules/include/shardrouter.h @@ -245,4 +245,6 @@ typedef struct router_instance { #define BACKEND_TYPE(b) (SERVER_IS_MASTER((b)->backend_server) ? BE_MASTER : \ (SERVER_IS_SLAVE((b)->backend_server) ? BE_SLAVE : BE_UNDEFINED)); +bool subsvc_is_valid(SUBSERVICE*); + #endif /*< _SHARDROUTER_H */ diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index be2fb68df..433cec8ef 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3554,7 +3554,6 @@ static rses_property_t* rses_property_init( prop->rses_prop_chk_tail = CHK_NUM_ROUTER_PROPERTY; #endif -return_prop: CHK_RSES_PROP(prop); return prop; } diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index 94ab35974..a0b2a4a43 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -319,7 +319,7 @@ parse_mapping_response(ROUTER_CLIENT_SES* rses, char* target, GWBUF* buf) if(PTR_IS_RESULTSET(((unsigned char*)buf->start)) && modutil_count_signal_packets(buf,0,0,&more) == 2) { - ptr = (char*)buf->start; + ptr = (unsigned char*)buf->start; if(ptr[5] != 1) {