From a41a8d6060c7b60e09686bea8124803f047d85ad Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 15 Sep 2014 19:01:04 +0300 Subject: [PATCH 1/5] Fix to bug #543, http://bugs.skysql.com/show_bug.cgi?id=543 All counters are now updated in routeQuery Fix to bug #545, http://bugs.skysql.com/show_bug.cgi?id=545 All sql variable and session modification statements, such as autocommit-, and set commands are routed to all nodes. --- query_classifier/query_classifier.cc | 11 +++++-- .../routing/readwritesplit/readwritesplit.c | 32 +++++++++++-------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 02d693ac1..f034d770f 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -454,7 +454,7 @@ static skygw_query_type_t resolve_query_type( /** SELECT ..INTO variable|OUTFILE|DUMPFILE */ if (lex->result != NULL) { - type = QUERY_TYPE_SESSION_WRITE; + type = QUERY_TYPE_GSYSVAR_WRITE; goto return_qtype; } @@ -543,7 +543,7 @@ static skygw_query_type_t resolve_query_type( else if (lex->sql_command == SQLCOM_SET_OPTION) { /** Either user- or system variable write */ - type |= QUERY_TYPE_SESSION_WRITE; + type |= QUERY_TYPE_GSYSVAR_WRITE; } goto return_qtype; } @@ -759,7 +759,12 @@ static skygw_query_type_t resolve_query_type( break; /** User-defined variable modification */ case Item_func::SUSERVAR_FUNC: - func_qtype |= QUERY_TYPE_SESSION_WRITE; + /** + * Really it is user variable but we + * don't separate sql variables atm. + * 15.9.14 + */ + func_qtype |= QUERY_TYPE_GSYSVAR_WRITE; LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [resolve_query_type] " diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index d7ba011bc..1e2b68517 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1209,13 +1209,15 @@ static route_target_t get_route_target ( /** * These queries are not affected by hints */ - if (!trx_active && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || + if (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || + QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || /** Configured to allow writing variables to all nodes */ (use_sql_variables_in == TYPE_ALL && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE))))) + QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE)) || + /** enable or disable autocommit are always routed to all */ + QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || + QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT)) { /** hints don't affect on routing */ target = TARGET_ALL; @@ -1678,8 +1680,6 @@ static int routeQuery( } goto retblock; } - inst->stats.n_queries++; - master_dcb = router_cli_ses->rses_master_ref->bref_dcb; CHK_DCB(master_dcb); @@ -1808,6 +1808,7 @@ static int routeQuery( if (succp) { + atomic_add(&inst->stats.n_all, 1); ret = 1; } goto retblock; @@ -1904,6 +1905,10 @@ static int routeQuery( BE_SLAVE, NULL, rlag_max); + if (succp) + { + atomic_add(&inst->stats.n_slave, 1); + } } if (!succp && TARGET_IS_MASTER(route_target)) @@ -1920,21 +1925,22 @@ static int routeQuery( { succp = true; } + atomic_add(&inst->stats.n_master, 1); target_dcb = master_dcb; } ss_dassert(succp); if (succp) /*< Have DCB of the target backend */ - { + { if ((ret = target_dcb->func.write(target_dcb, gwbuf_clone(querybuf))) == 1) { backend_ref_t* bref; - atomic_add(&inst->stats.n_slave, 1); - /** - * Add one query response waiter to backend reference - */ + atomic_add(&inst->stats.n_queries, 1); + /** + * Add one query response waiter to backend reference + */ bref = get_bref_from_dcb(router_cli_ses, target_dcb); bref_set_state(bref, BREF_QUERY_ACTIVE); bref_set_state(bref, BREF_WAITING_RESULT); @@ -3677,9 +3683,7 @@ static bool route_session_write( } /** Unlock router session */ rses_end_locked_router_action(router_cli_ses); - - atomic_add(&inst->stats.n_all, 1); - + return_succp: return succp; } From 3dc44ff6fd37c149ff7e8c6d08b4e14a4a489efe Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 15 Sep 2014 21:03:11 +0300 Subject: [PATCH 2/5] Fix to bug #544, http://bugs.skysql.com/show_bug.cgi?id=544 Changes to readwritesplit router. --- .../routing/readwritesplit/readwritesplit.c | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 1e2b68517..e239b72ab 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -613,25 +613,29 @@ createInstance(SERVICE *service, char **options) for (n = 0; router->servers[n]; n++) { int perc; + int wght; backend = router->servers[n]; - perc = (atoi(serverGetParameter( - backend->backend_server, - weightby)) * 1000) / total; - if (perc == 0) + wght = atoi(serverGetParameter(backend->backend_server, + weightby)); + perc = (wght*1000) / total; + + if (perc == 0 && wght != 0) + { perc = 1; + } backend->weight = perc; + if (perc == 0) { LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, + LOGFILE_ERROR, "Server '%s' has no value " "for weighting parameter '%s', " "no queries will be routed to " "this server.\n", - server->unique_name, + router->servers[n]->backend_server->unique_name, weightby))); } - } } } @@ -2452,7 +2456,10 @@ static bool select_connect_backend_servers( master_found = true; master_connected = true; /* assert with master_host */ - ss_dassert(master_host && ((*p_master_ref)->bref_backend->backend_server == master_host->backend_server) && SERVER_MASTER); + ss_dassert(master_host && + ((*p_master_ref)->bref_backend->backend_server == + master_host->backend_server) && + SERVER_MASTER); } /** New session or master failure case */ else @@ -2574,11 +2581,17 @@ static bool select_connect_backend_servers( * servers from the sorted list. First master found is selected. */ for (i=0; - iservers[i]->weight == 0) + { + continue; + } + if (SERVER_IS_RUNNING(b->backend_server) && ((b->backend_server->status & router->bitmask) == router->bitvalue)) From 7300e5878771558f17503f4225a2a72c120a9990 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 16 Sep 2014 12:11:08 +0300 Subject: [PATCH 3/5] Fix to #547, http://bugs.skysql.com/show_bug.cgi?id=547 readwritesplit.c:get_dcb now searches for master if slave is not available --- .../routing/readwritesplit/readwritesplit.c | 170 +++++++++++++----- 1 file changed, 126 insertions(+), 44 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index e239b72ab..3f3b8516f 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -268,6 +268,8 @@ static bool handle_error_new_connection( GWBUF* errmsg); static bool handle_error_reply_client(SESSION* ses, GWBUF* errmsg); +static backend_ref_t* get_root_master_bref(ROUTER_CLIENT_SES* rses); + static BACKEND* get_root_master( backend_ref_t* servers, int router_nservers); @@ -1044,6 +1046,7 @@ static bool get_dcb( int max_rlag) { backend_ref_t* backend_ref; + backend_ref_t* master_bref; int smallest_nconn = -1; int i; bool succp = false; @@ -1059,16 +1062,19 @@ static bool get_dcb( backend_ref = rses->rses_backend_ref; /** get root master from available servers */ + master_bref = get_root_master_bref(rses); +#if defined(SS_DEBUG) + master_host = get_root_master(backend_ref, rses->rses_nbackends); - + ss_dassert(master_bref->bref_backend == master_host); +#endif if (name != NULL) /*< Choose backend by name from a hint */ { ss_dassert(btype != BE_MASTER); /*< Master dominates and no name should be passed with it */ for (i=0; irses_nbackends; i++) { - BACKEND* b = backend_ref[i].bref_backend; - + BACKEND* b = backend_ref[i].bref_backend; /** * To become chosen: * backend must be in use, name must match, @@ -1081,10 +1087,10 @@ static bool get_dcb( name, b->backend_server->unique_name, PATH_MAX) == 0) && - master_host != NULL && + master_bref->bref_backend != NULL && (SERVER_IS_SLAVE(b->backend_server) || - SERVER_IS_RELAY_SERVER(b->backend_server) || - SERVER_IS_MASTER(b->backend_server))) + SERVER_IS_RELAY_SERVER(b->backend_server) || + SERVER_IS_MASTER(b->backend_server))) { *p_dcb = backend_ref[i].bref_dcb; succp = true; @@ -1113,15 +1119,15 @@ static bool get_dcb( * at the moment. */ if (BREF_IS_IN_USE((&backend_ref[i])) && - master_host != NULL && - b->backend_server != master_host->backend_server && + master_bref->bref_backend != NULL && + b->backend_server != master_bref->bref_backend->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)) && (SERVER_IS_SLAVE(b->backend_server) || - SERVER_IS_RELAY_SERVER(b->backend_server)) && + SERVER_IS_RELAY_SERVER(b->backend_server)) && (smallest_nconn == -1 || - b->backend_conn_count < smallest_nconn)) + b->backend_conn_count < smallest_nconn)) { *p_dcb = backend_ref[i].bref_dcb; smallest_nconn = b->backend_conn_count; @@ -1132,40 +1138,54 @@ static bool get_dcb( if (!succp) /*< No valid slave was found, search master next */ { + if (rses->router->available_slaves) + { + rses->router->available_slaves = false; + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Warning : No slaves available " + "for the service %s.", + rses->router->service->name))); + } + + btype = BE_MASTER; - - if (BREF_IS_IN_USE(backend_ref)) + + + if (BREF_IS_IN_USE(master_bref)) { - *p_dcb = backend_ref->bref_dcb; + *p_dcb = master_bref->bref_dcb; succp = true; - ss_dassert(backend_ref->bref_dcb->state != DCB_STATE_ZOMBIE); + ss_dassert(master_bref->bref_dcb->state != DCB_STATE_ZOMBIE); ss_dassert( - (master_host && (backend_ref->bref_backend->backend_server == master_host->backend_server)) && + (master_bref->bref_backend && + (master_bref->bref_backend->backend_server == + master_bref->bref_backend->backend_server)) && smallest_nconn == -1); - - if (rses->router->available_slaves) - { - rses->router->available_slaves = false; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Warning : No slaves available " - "for the service %s. " - "Using master %s:%d " - "instead.", - rses->router->service->name, - backend_ref->bref_backend->backend_server->name, - backend_ref->bref_backend->backend_server->port))); - } + + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Using master %s:%d instead.", + master_bref->bref_backend->backend_server->name, + master_bref->bref_backend->backend_server->port))); } + else + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : No master is availabe either. " + "Unable to find backend server for " + "routing."))); + } } else if (rses->router->available_slaves == false) { rses->router->available_slaves = true; LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "At least one slave has become avilable for " + "At least one slave has become available for " "the service %s.", rses->router->service->name))); } @@ -1179,7 +1199,9 @@ static bool get_dcb( BACKEND* b = backend_ref[i].bref_backend; if (BREF_IS_IN_USE((&backend_ref[i])) && - (master_host && (b->backend_server == master_host->backend_server))) + (master_bref->bref_backend && + (b->backend_server == + master_bref->bref_backend->backend_server))) { *p_dcb = backend_ref[i].bref_dcb; succp = true; @@ -2430,7 +2452,7 @@ static bool select_connect_backend_servers( const int min_nslaves = 0; /*< not configurable at the time */ bool is_synced_master; int (*p)(const void *, const void *); - BACKEND *master_host = NULL; + BACKEND* master_host = NULL; if (p_master_ref == NULL || backend_ref == NULL) { @@ -4305,22 +4327,82 @@ static bool prep_stmt_drop( * @return The Master found * */ -static BACKEND *get_root_master(backend_ref_t *servers, int router_nservers) { +static BACKEND *get_root_master( + backend_ref_t *servers, + int router_nservers) +{ int i = 0; BACKEND * master_host = NULL; - for (i = 0; i< router_nservers; i++) { - BACKEND* b = NULL; - b = servers[i].bref_backend; - if (b && (b->backend_server->status & (SERVER_MASTER|SERVER_MAINT)) == SERVER_MASTER) { - if (master_host && b->backend_server->depth < master_host->backend_server->depth) { - master_host = b; - } else { - if (master_host == NULL) { - master_host = b; - } + for (i = 0; i< router_nservers; i++) + { + BACKEND* b; + + if (servers[i].bref_backend == NULL) + { + continue; + } + + b = servers[i].bref_backend; + + if ((b->backend_server->status & + (SERVER_MASTER|SERVER_MAINT)) == SERVER_MASTER) + { + if (master_host == NULL || + (b->backend_server->depth < master_host->backend_server->depth)) + { + master_host = b; } } } return master_host; } + + +/******************************** + * This routine returns the root master server from MySQL replication tree + * Get the root Master rule: + * + * find server with the lowest replication depth level + * and the SERVER_MASTER bitval + * Servers are checked even if they are in 'maintenance' + * + * @param rses pointer to router session + * @return pointer to backend reference of the root master + * + */ +static backend_ref_t* get_root_master_bref( + ROUTER_CLIENT_SES* rses) +{ + backend_ref_t* bref; + backend_ref_t* candidate_bref = NULL; + int i = 0; + + bref = rses->rses_backend_ref; + + while (irses_nbackends) + { + if (SERVER_IS_MASTER(bref->bref_backend->backend_server)) + { + if (candidate_bref == NULL || + (bref->bref_backend->backend_server->depth < + candidate_bref->bref_backend->backend_server->depth)) + { + candidate_bref = bref; + } + } + bref++; + i += 1; + } + return candidate_bref; +} + + + + + + + + + + From 48c25155f578eed73849e7ebb13a94e0052e9012 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 16 Sep 2014 14:49:04 +0300 Subject: [PATCH 4/5] Completion to fix for http://bugs.skysql.com/show_bug.cgi?id=547 server.h:added macro SERVER_IS_ROOT_MASTER for finding valid candidate for root master readwritesplit.c: wrote open three if conditions in get_root_master_bref for clarity --- server/include/server.h | 8 ++++++++ .../routing/readwritesplit/readwritesplit.c | 14 +++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/server/include/server.h b/server/include/server.h index 81392243c..f6d146ccd 100644 --- a/server/include/server.h +++ b/server/include/server.h @@ -123,7 +123,15 @@ typedef struct server { */ #define SERVER_IS_MASTER(server) \ (((server)->status & (SERVER_RUNNING|SERVER_MASTER|SERVER_SLAVE|SERVER_MAINT)) == (SERVER_RUNNING|SERVER_MASTER)) + /** + * Is the server valid candidate for root master. The server must be running, + * marked as master and not have maintenance bit set. + */ +#define SERVER_IS_ROOT_MASTER(server) \ +(((server)->status & (SERVER_RUNNING|SERVER_MASTER|SERVER_MAINT)) == (SERVER_RUNNING|SERVER_MASTER)) + + /** * Is the server a slave? The server must be both running and marked as a slave * in order for the macro to return true */ diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 3f3b8516f..967f332a1 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -4382,13 +4382,17 @@ static backend_ref_t* get_root_master_bref( while (irses_nbackends) { - if (SERVER_IS_MASTER(bref->bref_backend->backend_server)) + if ((bref->bref_backend->backend_server->status & + (SERVER_MASTER|SERVER_MAINT)) == SERVER_MASTER) { - if (candidate_bref == NULL || - (bref->bref_backend->backend_server->depth < - candidate_bref->bref_backend->backend_server->depth)) + if (bref->bref_backend->backend_server->status & SERVER_MASTER) { - candidate_bref = bref; + if (candidate_bref == NULL || + (bref->bref_backend->backend_server->depth < + candidate_bref->bref_backend->backend_server->depth)) + { + candidate_bref = bref; + } } } bref++; From 3b452b47459a22f54ed0171472788494eb0bc566 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Tue, 16 Sep 2014 17:12:24 +0200 Subject: [PATCH 5/5] Documentation file names change Documentation file names change --- ... MaxScale Configuration And Usage Scenarios.pdf} | Bin ...df => MaxScale Debug And Diagnostic Support.pdf} | Bin ...etup-Z3.pdf => MaxScale MySQL Cluster setup.pdf} | Bin ... => RabbitMQ Setup And MaxScale Integration.pdf} | Bin 4 files changed, 0 insertions(+), 0 deletions(-) rename Documentation/{MaxScale Configuration And Usage Scenarios-Z3.pdf => MaxScale Configuration And Usage Scenarios.pdf} (100%) rename Documentation/{MaxScale Debug And Diagnostic Support-Z3.pdf => MaxScale Debug And Diagnostic Support.pdf} (100%) rename Documentation/{MaxScale MySQL Cluster setup-Z3.pdf => MaxScale MySQL Cluster setup.pdf} (100%) rename Documentation/{RabbitMQ Setup And MaxScale Integration-Z3.pdf => RabbitMQ Setup And MaxScale Integration.pdf} (100%) diff --git a/Documentation/MaxScale Configuration And Usage Scenarios-Z3.pdf b/Documentation/MaxScale Configuration And Usage Scenarios.pdf similarity index 100% rename from Documentation/MaxScale Configuration And Usage Scenarios-Z3.pdf rename to Documentation/MaxScale Configuration And Usage Scenarios.pdf diff --git a/Documentation/MaxScale Debug And Diagnostic Support-Z3.pdf b/Documentation/MaxScale Debug And Diagnostic Support.pdf similarity index 100% rename from Documentation/MaxScale Debug And Diagnostic Support-Z3.pdf rename to Documentation/MaxScale Debug And Diagnostic Support.pdf diff --git a/Documentation/MaxScale MySQL Cluster setup-Z3.pdf b/Documentation/MaxScale MySQL Cluster setup.pdf similarity index 100% rename from Documentation/MaxScale MySQL Cluster setup-Z3.pdf rename to Documentation/MaxScale MySQL Cluster setup.pdf diff --git a/Documentation/RabbitMQ Setup And MaxScale Integration-Z3.pdf b/Documentation/RabbitMQ Setup And MaxScale Integration.pdf similarity index 100% rename from Documentation/RabbitMQ Setup And MaxScale Integration-Z3.pdf rename to Documentation/RabbitMQ Setup And MaxScale Integration.pdf