From ebd92c87417844b282ae61cc314251031359ef59 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 20 Oct 2014 10:40:32 +0300 Subject: [PATCH 1/5] readwritesplit.c:routeQuery if target is master but it is found out when get_dcb returns that master DCB has changed, routeQuery fails and logs to trace that master changed. --- .../routing/readwritesplit/readwritesplit.c | 68 ++++++++++++------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 5a239a5ef..a2b093631 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -849,7 +849,7 @@ static void* newSession( free(client_rses); client_rses = NULL; goto return_rses; - } + } /** Copy backend pointers to router session. */ client_rses->rses_master_ref = master_ref; /* assert with master_host */ @@ -1065,9 +1065,12 @@ static bool get_dcb( /** 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); + if (master_bref->bref_backend != master_host) + { + LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, + "Master has changed."))); + } #endif if (name != NULL) /*< Choose backend by name from a hint */ { @@ -1714,6 +1717,7 @@ static int routeQuery( querybuf = gwbuf_make_contiguous(querybuf); } + /** Read stored master DCB pointer */ master_dcb = router_cli_ses->rses_master_ref->bref_dcb; CHK_DCB(master_dcb); @@ -2000,30 +2004,43 @@ static int routeQuery( if (!succp && TARGET_IS_MASTER(route_target)) { - if (master_dcb == NULL) + DCB* curr_master_dcb; + + succp = get_dcb(&curr_master_dcb, + router_cli_ses, + BE_MASTER, + NULL, + MAX_RLAG_UNDEFINED); + + if (succp && (master_dcb == NULL || master_dcb == curr_master_dcb)) { - succp = get_dcb(&master_dcb, - router_cli_ses, - BE_MASTER, - NULL, - MAX_RLAG_UNDEFINED); - if (!succp) + atomic_add(&inst->stats.n_master, 1); + target_dcb = master_dcb; + } + else + { + if (succp && master_dcb != curr_master_dcb) { LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, "Was supposed to " "route to master " - "but couldn't find " - "master in a " - "suitable state " - "failed."))); + "but master has " + "changed."))); } + else + { + LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, + "Was supposed to " + "route to master " + "but couldn't find " + "master in a " + "suitable state " + "failed."))); + } + succp = false; + ret = 0; } - else - { - succp = true; - } - atomic_add(&inst->stats.n_master, 1); - target_dcb = master_dcb; + } if (succp) /*< Have DCB of the target backend */ @@ -2595,7 +2612,7 @@ static bool select_connect_backend_servers( } /* get the root Master */ - master_host = get_root_master(backend_ref, router_nservers); + master_host = get_root_master(backend_ref, router_nservers); /** Master is already chosen and connected. This is slave failure case */ if (*p_master_ref != NULL && @@ -2610,11 +2627,16 @@ static bool select_connect_backend_servers( master_found = true; master_connected = true; - /* assert with master_host */ + + /** + * Ensure that *p_master_ref and master_host point to same backend + * and it has a master role. + */ ss_dassert(master_host && ((*p_master_ref)->bref_backend->backend_server == master_host->backend_server) && - SERVER_MASTER); + (master_host->backend_server->status & + (SERVER_MASTER|SERVER_MAINT)) == SERVER_MASTER); } /** New session or master failure case */ else From 4daf255d608816ade499dcf8734af2714e1ea20f Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 20 Oct 2014 13:20:53 +0300 Subject: [PATCH 2/5] Fixed use of uninitialized variable. --- 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 a2b093631..8e3ca8aeb 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -2004,7 +2004,7 @@ static int routeQuery( if (!succp && TARGET_IS_MASTER(route_target)) { - DCB* curr_master_dcb; + DCB* curr_master_dcb = NULL; succp = get_dcb(&curr_master_dcb, router_cli_ses, From d065be482418dfe0b6611a9f4d12222b60ad80ef Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 20 Oct 2014 22:41:10 +0300 Subject: [PATCH 3/5] readwritesplit.c:get_dcb assumed thet get_root_master_bref always returns non-null pointer. Changed it so that get_dcb_returns if it doesn't get master bref pointer. --- server/modules/routing/readwritesplit/readwritesplit.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 8e3ca8aeb..9bc1e38c3 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1064,6 +1064,14 @@ static bool get_dcb( /** get root master from available servers */ master_bref = get_root_master_bref(rses); + /** + * If master can't be found, session will be closed. + */ + if (master_bref == NULL) + { + succp = false; + goto return_succp; + } #if defined(SS_DEBUG) master_host = get_root_master(backend_ref, rses->rses_nbackends); if (master_bref->bref_backend != master_host) From 502c78de2b73974c79e68ad301ca918b69256fa3 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Tue, 21 Oct 2014 15:30:32 +0100 Subject: [PATCH 4/5] Add swp files --- .gitignore | 1 + server/core/secrets.c | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 6e659cc6b..c761d00c4 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,7 @@ depend.mk *~ *# .#* +._* # Vi swap files .*.swp diff --git a/server/core/secrets.c b/server/core/secrets.c index c0b82c347..3e57591ed 100644 --- a/server/core/secrets.c +++ b/server/core/secrets.c @@ -65,6 +65,7 @@ MAXKEYS *keys; struct stat secret_stats; int fd; int len; +static int reported = 0; home = getenv("MAXSCALE_HOME"); @@ -80,12 +81,16 @@ int len; errno = 0; if (eno == ENOENT) { - LOGIF(LM, (skygw_log_write( + if (!reported) + { + LOGIF(LM, (skygw_log_write( LOGFILE_MESSAGE, "Encrypted password file %s can't be accessed " "(%s). Password encryption is not used.", secret_file, strerror(eno)))); + reported = 1; + } } else { From 761de0ac8ab7a05a099bc2b23699df481dc3ce29 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Tue, 21 Oct 2014 17:51:11 +0200 Subject: [PATCH 5/5] Removed possible uninitialised pointer value Removed possible uninitialised pointer value in host parsing for wildcard --- server/core/dbusers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dbusers.c b/server/core/dbusers.c index 3b2b36361..69edc4acb 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -200,7 +200,7 @@ int add_mysql_users_with_host_ipv4(USERS *users, char *user, char *host, char *p tmp = ret_ip+strlen(ret_ip)-1; /* start from Class C */ - while(*tmp) { + while(tmp > ret_ip) { if (*tmp == '%') { /* set only the last IPv4 byte to 1 * avoiding setipadress() failure