From bac6795105f2baf7dd852b218a85316fa7ff8285 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 2 Dec 2014 15:33:17 +0200 Subject: [PATCH 1/9] Added log synchronization to disk when signals are received. --- log_manager/log_manager.cc | 77 ++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 92eaaff0f..037095aba 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -94,7 +94,9 @@ char* syslog_ident_str = NULL; */ static int lmlock; static logmanager_t* lm; - +static bool flushall_flag; +static bool flushall_started_flag; +static bool flushall_done_flag; /** Writer thread structure */ struct filewriter_st { @@ -291,7 +293,8 @@ static bool check_file_and_path( static bool file_is_symlink(char* filename); static int skygw_log_disable_raw(logfile_id_t id, bool emergency); /*< no locking */ static int find_last_seqno(strpart_t* parts, int seqno, int seqnoidx); - +void flushall_logfiles(bool flush); +bool thr_flushall_check(); const char* get_suffix_default(void) { @@ -2802,13 +2805,15 @@ static void* thr_filewriter_fun( int i; blockbuf_state_t flush_blockbuf; /**< flush single block buffer. */ bool flush_logfile; /**< flush logfile */ - bool flushall_logfiles;/**< flush all logfiles */ + bool do_flushall = false; bool rotate_logfile; /*< close current and open new file */ size_t vn1; size_t vn2; thr = (skygw_thread_t *)data; fwr = (filewriter_t *)skygw_thread_get_data(thr); + flushall_logfiles(false); + CHK_FILEWRITER(fwr); ss_debug(skygw_thread_set_state(thr, THR_RUNNING)); @@ -2821,8 +2826,9 @@ static void* thr_filewriter_fun( * Reset message to avoid redundant calls. */ skygw_message_wait(fwr->fwr_logmes); - - flushall_logfiles = skygw_thread_must_exit(thr); + if(skygw_thread_must_exit(thr)){ + flushall_logfiles(true); + } /** Process all logfiles which have buffered writes. */ for (i=LOGFILE_FIRST; i<=LOGFILE_LAST; i <<= 1) @@ -2831,6 +2837,10 @@ static void* thr_filewriter_fun( /** * Get file pointer of current logfile. */ + + + + do_flushall = thr_flushall_check(); file = fwr->fwr_file[i]; lf = &lm->lm_logfile[(logfile_id_t)i]; @@ -2901,7 +2911,7 @@ static void* thr_filewriter_fun( if (bb->bb_buf_used != 0 && (flush_blockbuf == BB_FULL || flush_logfile || - flushall_logfiles)) + do_flushall)) { /** * buffer is at least half-full @@ -2920,7 +2930,7 @@ static void* thr_filewriter_fun( (void *)bb->bb_buf, bb->bb_buf_used, (flush_logfile || - flushall_logfiles)); + do_flushall)); if (err) { fprintf(stderr, @@ -2967,13 +2977,28 @@ static void* thr_filewriter_fun( * Loop is restarted to ensure that all logfiles are * flushed. */ - if (!flushall_logfiles && skygw_thread_must_exit(thr)) + + if(flushall_started_flag){ + flushall_started_flag = false; + flushall_done_flag = true; + i = LOGFILE_FIRST; + goto retry_flush_on_exit; + } + + if (!thr_flushall_check() && skygw_thread_must_exit(thr)) { - flushall_logfiles = true; + flushall_logfiles(true); i = LOGFILE_FIRST; goto retry_flush_on_exit; } - } /* for */ + }/* for */ + + if(flushall_done_flag){ + flushall_done_flag = false; + flushall_logfiles(false); + skygw_message_send(fwr->fwr_clientmes); + } + } /* while (!skygw_thread_must_exit) */ ss_debug(skygw_thread_set_state(thr, THR_STOPPED)); @@ -3073,4 +3098,34 @@ static int find_last_seqno( free(snstr); return seqno; -} \ No newline at end of file +} + +bool thr_flushall_check() +{ + bool rval = false; + simple_mutex_lock(&lm->lm_mutex,true); + rval = flushall_flag; + if(rval && !flushall_started_flag && !flushall_done_flag){ + flushall_started_flag = true; + } + simple_mutex_unlock(&lm->lm_mutex); + return rval; +} + +void flushall_logfiles(bool flush) +{ + simple_mutex_lock(&lm->lm_mutex,true); + flushall_flag = flush; + simple_mutex_unlock(&lm->lm_mutex); +} + +/** + * Flush all log files synchronously + */ +void skygw_log_sync_all(void) +{ + skygw_log_write(LOGFILE_TRACE,"Starting log flushing to disk."); + flushall_logfiles(true); + skygw_message_send(lm->lm_logmes); + skygw_message_wait(lm->lm_clientmes); +} From ad2ca6224821e56676bfeea1ddf239c62a7bfb9e Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Tue, 2 Dec 2014 13:39:46 +0000 Subject: [PATCH 2/9] Log the start of the lsitener to the message log in line with other protocols --- server/modules/protocol/maxscaled.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index 1c564da56..1bf009d97 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -374,8 +374,8 @@ int rc; rc = listen(listener->fd, SOMAXCONN); if (rc == 0) { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, + LOGIF(LM, (skygw_log_write( + LOGFILE_MESSAGE, "Listening maxscale connections at %s\n", config))); } else { From aa8350ef8fe3f6d295394ed2acfe213f43490b5d Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Tue, 2 Dec 2014 18:47:25 +0100 Subject: [PATCH 3/9] client's flags copied among known capabilities Copy client's flags to backend but with the known capabilities mask --- server/modules/protocol/mysql_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 27159dc21..364b13595 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -577,8 +577,8 @@ int gw_send_authentication_to_backend( dcb = conn->owner_dcb; final_capabilities = gw_mysql_get_byte4((uint8_t *)&server_capabilities); - /** Copy client's flags to backend */ - final_capabilities |= conn->client_capabilities; + /** Copy client's flags to backend but with the known capabilities mask */ + final_capabilities |= (conn->client_capabilities & GW_MYSQL_CAPABILITIES_CLIENT); /* get charset the client sent and use it for connection auth */ charset = conn->charset; From d608eb2532b1dfc3e1395abdc58c9966f9aec35e Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 2 Dec 2014 20:02:00 +0200 Subject: [PATCH 4/9] Fix to bug #622, http://bugs.skysql.com/show_bug.cgi?id=622 Ensured that soft link and physical file will have same sequence number. Prevented some unnecessary error printing and added more precise logs. --- log_manager/log_manager.cc | 102 +++++++++++++++++++++++++---------- server/MaxScale_template.cnf | 10 +++- 2 files changed, 84 insertions(+), 28 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 92eaaff0f..fc7db4117 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -286,7 +286,8 @@ static char* add_slash(char* str); static bool check_file_and_path( char* filename, - bool* writable); + bool* writable, + bool do_log); static bool file_is_symlink(char* filename); static int skygw_log_disable_raw(logfile_id_t id, bool emergency); /*< no locking */ @@ -2053,7 +2054,7 @@ static bool logfile_create( * If file exists but is different type, create fails and * new, increased sequence number is added to file name. */ - if (check_file_and_path(lf->lf_full_file_name, &writable)) + if (check_file_and_path(lf->lf_full_file_name, &writable, true)) { /** Found similarly named file which isn't writable */ if (!writable || file_is_symlink(lf->lf_full_file_name)) @@ -2077,13 +2078,12 @@ static bool logfile_create( if (store_shmem) { - if (check_file_and_path(lf->lf_full_file_name, &writable)) + if (check_file_and_path(lf->lf_full_link_name, &writable, true)) { - /** Found similarly named file which isn't writable */ - if (!writable || - file_is_symlink(lf->lf_full_file_name)) + /** Found similarly named link which isn't writable */ + if (!writable) { - unlink(lf->lf_full_file_name); + unlink(lf->lf_full_link_name); nameconflicts = true; } } @@ -2214,7 +2214,6 @@ return_succp: * * @return Pointer to filename, of NULL if failed. * - * */ static char* form_full_file_name( strpart_t* parts, @@ -2231,9 +2230,21 @@ static char* form_full_file_name( if (lf->lf_name_seqno != -1) { - lf->lf_name_seqno = find_last_seqno(parts, - lf->lf_name_seqno, - seqnoidx); + int file_sn; + int link_sn = 0; + char* tmp = parts[0].sp_string; + + file_sn = find_last_seqno(parts, lf->lf_name_seqno, seqnoidx); + + if (lf->lf_linkpath != NULL) + { + tmp = parts[0].sp_string; + parts[0].sp_string = lf->lf_linkpath; + link_sn = find_last_seqno(parts, lf->lf_name_seqno, seqnoidx); + parts[0].sp_string = tmp; + } + lf->lf_name_seqno = MAX(file_sn, link_sn); + seqno = lf->lf_name_seqno; s = UINTLEN(seqno); seqnostr = (char *)malloc((int)s+1); @@ -2354,7 +2365,8 @@ static char* add_slash( */ static bool check_file_and_path( char* filename, - bool* writable) + bool* writable, + bool do_log) { int fd; bool exists; @@ -2382,11 +2394,23 @@ static bool check_file_and_path( if (fd == -1) { - fprintf(stderr, - "*\n* Error : Can't access %s due " - "to %s.\n", - filename, - strerror(errno)); + if (do_log && file_is_symlink(filename)) + { + fprintf(stderr, + "*\n* Error : Can't access " + "file pointed to by %s due " + "to %s.\n", + filename, + strerror(errno)); + } + else if (do_log) + { + fprintf(stderr, + "*\n* Error : Can't access %s due " + "to %s.\n", + filename, + strerror(errno)); + } if (writable) { *writable = false; @@ -2403,11 +2427,24 @@ static bool check_file_and_path( } else { - fprintf(stderr, - "*\n* Error : Can't write to " - "%s due to %s.\n", - filename, - strerror(errno)); + if (do_log && + file_is_symlink(filename)) + { + fprintf(stderr, + "*\n* Error : Can't write to " + "file pointed to by %s due to " + "%s.\n", + filename, + strerror(errno)); + } + else if (do_log) + { + fprintf(stderr, + "*\n* Error : Can't write to " + "%s due to %s.\n", + filename, + strerror(errno)); + } *writable = false; } } @@ -2417,10 +2454,21 @@ static bool check_file_and_path( } else { - fprintf(stderr, - "*\n* Error : Can't access %s due to %s.\n", - filename, - strerror(errno)); + if (do_log && file_is_symlink(filename)) + { + fprintf(stderr, + "*\n* Error : Can't access the file " + "pointed to by %s due to %s.\n", + filename, + strerror(errno)); + } + else if (do_log) + { + fprintf(stderr, + "*\n* Error : Can't access %s due to %s.\n", + filename, + strerror(errno)); + } exists = false; if (writable) @@ -3061,7 +3109,7 @@ static int find_last_seqno( } } - if (check_file_and_path(filename, NULL)) + if (check_file_and_path(filename, NULL, false)) { seqno++; } diff --git a/server/MaxScale_template.cnf b/server/MaxScale_template.cnf index cb49c3e14..0a6d22a88 100644 --- a/server/MaxScale_template.cnf +++ b/server/MaxScale_template.cnf @@ -31,7 +31,7 @@ threads=4 # backend_write_timeout= # backend_read_timeout= # -## mysql_monitor specific options: +## MySQL monitor-specific options: # # Enable detection of replication slaves lag via replication_heartbeat # table - optional. @@ -43,6 +43,13 @@ threads=4 # # detect_stale_master=[1|0] (default 0) # +## Galera monitor-specific options: +# +# If disable_master_failback is not set, recovery of previously failed master +# causes mastership to be switched back to it. Enabling the option prevents it. +# +# disable_master_failback=[0|1] (default 0) +# ## Examples: [MySQL Monitor] @@ -65,6 +72,7 @@ servers=server1,server2,server3 user=myuser passwd=mypwd monitor_interval=10000 +disable_master_failback=1 ## Filter definition # From aab30f2eeacc9b7e66e344fcc1c5f38c7c3f3d68 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 2 Dec 2014 23:15:24 +0200 Subject: [PATCH 5/9] If linked log file in /dev/shm wasn't writable a new log file with increased seq.no. was created but the old link was also deleted. This was ok before log rotation and file creation with increasing seq.no was implemented but useless and confusing today. Enabled printing this also in Release version: raatikka@linux-yxkl:~/bin/develop/bin> ./maxscale -d -c ../ Info : MaxScale will be run in the terminal process. Using Home directory command-line argument as MAXSCALE_HOME = /home/raatikka/bin/develop Error log : /home/raatikka/bin/develop/log/skygw_err1.log Message log : /home/raatikka/bin/develop/log/skygw_msg1.log Trace log : /home/raatikka/bin/develop/log/skygw_trace1.log->/dev/shm/7886/skygw_trace1.log Debug log : /home/raatikka/bin/develop/log/skygw_debug1.log->/dev/shm/7886/skygw_debug1.log Home directory : /home/raatikka/bin/develop Configuration file : /home/raatikka/bin/develop/etc/MaxScale.cnf Log directory : /home/raatikka/bin/develop/log Data directory : /home/raatikka/bin/develop/data/data7886 That is, all log files' names and locations are printed on the screen. --- log_manager/log_manager.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 000d7cd08..52c53f420 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -2086,7 +2086,6 @@ static bool logfile_create( /** Found similarly named link which isn't writable */ if (!writable) { - unlink(lf->lf_full_link_name); nameconflicts = true; } } @@ -2623,7 +2622,7 @@ static bool logfile_init( logfile_free_memory(logfile); goto return_with_succp; } -#if defined(SS_DEBUG) + if (store_shmem) { fprintf(stderr, "%s\t: %s->%s\n", @@ -2637,7 +2636,6 @@ static bool logfile_init( STRLOGNAME(logfile_id), logfile->lf_full_file_name); } -#endif succp = true; logfile->lf_state = RUN; CHK_LOGFILE(logfile); From f2c2e4f81cefef2e99db4d30d079515ff3ce7ec7 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 2 Dec 2014 23:58:28 +0200 Subject: [PATCH 6/9] Fix to bug #617, http://bugs.skysql.com/show_bug.cgi?id=617 When filewriter_init failed due to full disk, error branch freed messages twice. Removed unnecessary free command. --- log_manager/log_manager.cc | 11 ----------- utils/skygw_utils.cc | 10 +++++----- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 52c53f420..230e44804 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -445,17 +445,6 @@ static bool logmanager_init_nomutex( return_succp: if (err != 0) { - if (lm != NULL) - { - if (lm->lm_clientmes != NULL) - { - skygw_message_done(lm->lm_clientmes); - } - if (lm->lm_logmes != NULL) - { - skygw_message_done(lm->lm_logmes); - } - } /** This releases memory of all created objects */ logmanager_done_nomutex(); fprintf(stderr, "*\n* Error : Initializing log manager failed.\n*\n"); diff --git a/utils/skygw_utils.cc b/utils/skygw_utils.cc index 8e285c6a2..fd25c60cf 100644 --- a/utils/skygw_utils.cc +++ b/utils/skygw_utils.cc @@ -1680,12 +1680,12 @@ static bool file_write_header( if (wbytes1 != 1 || wbytes2 != 1 || wbytes3 != 1 || wbytes4 != 1) { fprintf(stderr, - "* Writing header %s %s %s to %s failed.\n", + "\nError : Writing header %s %s %s %s failed.\n", header_buf1, header_buf2, header_buf3, header_buf4); - perror("Logfile header write.\n"); + perror("Logfile header write"); goto return_succp; } #endif @@ -1757,11 +1757,11 @@ static bool file_write_footer( if (wbytes1 != 1 || wbytes3 != 1 || wbytes4 != 1) { fprintf(stderr, - "* Writing header %s %s to %s failed.\n", + "\nError : Writing header %s %s to %s failed.\n", header_buf1, header_buf3, header_buf4); - perror("Logfile header write.\n"); + perror("Logfile header write"); goto return_succp; } #endif @@ -1875,7 +1875,7 @@ skygw_file_t* skygw_file_init( int eno = errno; errno = 0; fprintf(stderr, - "* Writing header of log file %s failed due %d, %s.\n", + "\nError : Writing header of log file %s failed due %d, %s.\n", file->sf_fname, eno, strerror(eno)); From 6c3d27fd8f6aec33db3708365d82f2898fee57dd Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 3 Dec 2014 10:09:29 +0200 Subject: [PATCH 7/9] Commented out optional Galera monitor option disable_master_failback . --- server/MaxScale_template.cnf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/MaxScale_template.cnf b/server/MaxScale_template.cnf index 0a6d22a88..a3f44ebf1 100644 --- a/server/MaxScale_template.cnf +++ b/server/MaxScale_template.cnf @@ -72,7 +72,7 @@ servers=server1,server2,server3 user=myuser passwd=mypwd monitor_interval=10000 -disable_master_failback=1 +#disable_master_failback= ## Filter definition # From 7079cb47494ecc05e7470bf9596340386951cecc Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 3 Dec 2014 10:35:34 +0200 Subject: [PATCH 8/9] Cleaned a bit --- server/modules/include/dbshard.h | 7 +- server/modules/routing/dbshard/dbshard.c | 146 +++-------------------- 2 files changed, 16 insertions(+), 137 deletions(-) diff --git a/server/modules/include/dbshard.h b/server/modules/include/dbshard.h index e44026d72..ee23da273 100644 --- a/server/modules/include/dbshard.h +++ b/server/modules/include/dbshard.h @@ -75,11 +75,8 @@ struct router_instance; typedef enum { TARGET_UNDEFINED = 0x00, - TARGET_MASTER = 0x01, - TARGET_SLAVE = 0x02, - TARGET_NAMED_SERVER = 0x04, - TARGET_ALL = 0x08, - TARGET_RLAG_MAX = 0x10 + TARGET_SINGLE = 0x01, + TARGET_ALL = 0x02 } route_target_t; #define TARGET_IS_MASTER(t) (t & TARGET_MASTER) diff --git a/server/modules/routing/dbshard/dbshard.c b/server/modules/routing/dbshard/dbshard.c index 6b4ebea0c..62f1fb6e1 100644 --- a/server/modules/routing/dbshard/dbshard.c +++ b/server/modules/routing/dbshard/dbshard.c @@ -1344,9 +1344,9 @@ static backend_ref_t* check_candidate_bref( */ static route_target_t get_route_target ( skygw_query_type_t qtype, - bool trx_active, - target_t use_sql_variables_in, - HINT* hint) + bool trx_active, /*< !!! turha ? */ + target_t use_sql_variables_in, /*< 'master' == single tässä tapauksessa */ + HINT* hint) /*< !!! turha ? */ { route_target_t target = TARGET_UNDEFINED; /** @@ -1365,136 +1365,9 @@ static route_target_t get_route_target ( /** hints don't affect on routing */ target = TARGET_ALL; } - /** - * Hints may affect on routing of the following queries - */ - else if (!trx_active && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || /*< any SELECT */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ)|| /*< read user var */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || /*< read sys var */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || /*< prepared stmt exec */ - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ))) /*< read global sys var */ - { - /** First set expected targets before evaluating hints */ - if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SHOW_TABLES) || /*< 'SHOW TABLES' */ - /** Configured to allow reading variables from slaves */ - (use_sql_variables_in == TYPE_ALL && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ)))) - { - target = TARGET_SLAVE; - } - else if (QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || - /** Configured not to allow reading variables from slaves */ - (use_sql_variables_in == TYPE_MASTER && - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ)))) - { - target = TARGET_MASTER; - } - /** process routing hints */ - while (hint != NULL) - { - if (hint->type == HINT_ROUTE_TO_MASTER) - { - target = TARGET_MASTER; /*< override */ - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [get_route_target] Hint: route to master.", - pthread_self()))); - break; - } - else if (hint->type == HINT_ROUTE_TO_NAMED_SERVER) - { - /** - * Searching for a named server. If it can't be - * found, the oroginal target is chosen. - */ - target |= TARGET_NAMED_SERVER; - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [get_route_target] Hint: route to " - "named server : ", - pthread_self()))); - } - else if (hint->type == HINT_ROUTE_TO_UPTODATE_SERVER) - { - /** not implemented */ - } - else if (hint->type == HINT_ROUTE_TO_ALL) - { - /** not implemented */ - } - else if (hint->type == HINT_PARAMETER) - { - if (strncasecmp( - (char *)hint->data, - "max_slave_replication_lag", - strlen("max_slave_replication_lag")) == 0) - { - target |= TARGET_RLAG_MAX; - } - else - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Error : Unknown hint parameter " - "'%s' when 'max_slave_replication_lag' " - "was expected.", - (char *)hint->data))); - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unknown hint parameter " - "'%s' when 'max_slave_replication_lag' " - "was expected.", - (char *)hint->data))); - } - } - else if (hint->type == HINT_ROUTE_TO_SLAVE) - { - target = TARGET_SLAVE; - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [get_route_target] Hint: route to " - "slave.", - pthread_self()))); - } - hint = hint->next; - } /*< while (hint != NULL) */ - /** If nothing matches then choose the master */ - if ((target & (TARGET_ALL|TARGET_SLAVE|TARGET_MASTER)) == 0) - { - target = TARGET_MASTER; - } - } else { - /** hints don't affect on routing */ - ss_dassert(trx_active || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_WRITE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_SESSION_WRITE) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) && - use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) && - use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ) && - use_sql_variables_in == TYPE_MASTER) || - (QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_WRITE) && - use_sql_variables_in == TYPE_MASTER) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_BEGIN_TRX) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_ROLLBACK) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_COMMIT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_CREATE_TMP_TABLE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_READ_TMP_TABLE) || - QUERY_IS_TYPE(qtype, QUERY_TYPE_UNKNOWN))); - target = TARGET_MASTER; + target = TARGET_SINGLE; } #if defined(SS_DEBUG) LOGIF(LT, (skygw_log_write( @@ -1815,7 +1688,6 @@ static int routeQuery( bool rses_is_closed = false; route_target_t route_target; bool succp = false; - int rlag_max = MAX_RLAG_UNDEFINED; backend_type_t btype; /*< target backend type */ CHK_CLIENT_RSES(router_cli_ses); @@ -1904,6 +1776,9 @@ static int routeQuery( break; } /**< switch by packet type */ + /** + * !!! Temporary tablen tutkiminen voi olla turhaa. Poista tarvittaessa. + */ /** * Check if the query has anything to do with temporary tables. */ @@ -1911,6 +1786,10 @@ static int routeQuery( check_create_tmp_table(instance,router_session,querybuf,qtype); check_drop_tmp_table(instance,router_session,querybuf,qtype); + /** + * !!! Transaktion tutkiminen voi olla turhaa paitsi jos haluataan + * lokittaa. Poista tarvittaessa. + */ /** * If autocommit is disabled or transaction is explicitly started * transaction becomes active and master gets all statements until @@ -1973,6 +1852,9 @@ static int routeQuery( free(contentstr); free(qtypestr); } + /** + * Find out whether the query should be routed to single server or to + * all of them. /** * Find out where to route the query. Result may not be clear; it is * possible to have a hint for routing to a named server which can From 68fc849c6efdb988e9d9e24310f9bc23b204d087 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Sun, 7 Dec 2014 00:34:23 +0200 Subject: [PATCH 9/9] query_classifier.cc:cleaned up some intendentions and brackets. dbshard.h:removed unnecessary code dbshard.c:removed unnecessary code, cleaned up a bit and made some required changes readwritesplit.c:removed two unnecessary variable assignments. --- query_classifier/query_classifier.cc | 50 +- server/modules/include/dbshard.h | 33 +- server/modules/routing/dbshard/dbshard.c | 925 ++++++------------ .../routing/readwritesplit/readwritesplit.c | 2 - 4 files changed, 329 insertions(+), 681 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index e742a5e94..b4bb9ac7a 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -1441,47 +1441,55 @@ char* skygw_get_qtype_str( /** * Returns an array of strings of databases that this query uses. - * If the database isn't defined in the query, it is assumed that this query only targets the current database. - * The value of @p size is set to the number of allocated strings. The caller is responsible for freeing all the allocated memory. + * If the database isn't defined in the query, it is assumed that this query + * only targets the current database. + * The value of @p size is set to the number of allocated strings. The caller is + * responsible for freeing all the allocated memory. * @param querybuf GWBUF containing the query * @param size Size of the resulting array - * @return A new array of strings containing the database names or NULL if no databases were found. + * @return A new array of strings containing the database names or NULL if no + * databases were found. */ -char** skygw_get_database_names(GWBUF* querybuf,int* size) +char** skygw_get_database_names(GWBUF* querybuf,int* size) { LEX* lex; TABLE_LIST* tbl; char **databases = NULL, **tmp = NULL; - int currsz = 0,i = 0; + int currsz = 0,i = 0; - if( (lex = get_lex(querybuf)) == NULL) + if( (lex = get_lex(querybuf)) == NULL) { - goto retblock; - } + goto retblock; + } lex->current_select = lex->all_selects_list; - - while(lex->current_select){ - tbl = lex->current_select->join_list->head(); - while(tbl) + while(lex->current_select) + { + tbl = lex->current_select->join_list->head(); + while(tbl) { - if(strcmp(tbl->db,"skygw_virtual") != 0){ - if(i>= currsz){ - tmp = (char**)realloc(databases,sizeof(char*)*(currsz*2 + 1)); - if(tmp == NULL) goto retblock; + if(strcmp(tbl->db,"skygw_virtual") != 0) + { + if(i>= currsz) + { + tmp = (char**)realloc(databases, + sizeof(char*)*(currsz*2 + 1)); + if(tmp == NULL) + { + goto retblock; + } databases = tmp; currsz = currsz*2 + 1; - } + } databases[i++] = strdup(tbl->db); } tbl=tbl->next_local; } - - lex->current_select = lex->current_select->next_select_in_list(); - } + lex->current_select = lex->current_select->next_select_in_list(); + } - retblock: +retblock: *size = i; return databases; } diff --git a/server/modules/include/dbshard.h b/server/modules/include/dbshard.h index ee23da273..3ec0dc947 100644 --- a/server/modules/include/dbshard.h +++ b/server/modules/include/dbshard.h @@ -75,15 +75,16 @@ struct router_instance; typedef enum { TARGET_UNDEFINED = 0x00, - TARGET_SINGLE = 0x01, - TARGET_ALL = 0x02 + TARGET_MASTER = 0x01, + TARGET_SLAVE = 0x02, + TARGET_NAMED_SERVER = 0x04, + TARGET_ALL = 0x08, + TARGET_RLAG_MAX = 0x10 } route_target_t; -#define TARGET_IS_MASTER(t) (t & TARGET_MASTER) -#define TARGET_IS_SLAVE(t) (t & TARGET_SLAVE) + #define TARGET_IS_NAMED_SERVER(t) (t & TARGET_NAMED_SERVER) #define TARGET_IS_ALL(t) (t & TARGET_ALL) -#define TARGET_IS_RLAG_MAX(t) (t & TARGET_RLAG_MAX) typedef struct rses_property_st rses_property_t; typedef struct router_client_session ROUTER_CLIENT_SES; @@ -97,24 +98,6 @@ typedef enum rses_property_type_t { RSES_PROP_TYPE_COUNT=RSES_PROP_TYPE_LAST+1 } rses_property_type_t; - - -/** - * This criteria is used when backends are chosen for a router session's use. - * Backend servers are sorted to ascending order according to the criteria - * and top N are chosen. - */ -typedef enum select_criteria { - UNDEFINED_CRITERIA=0, - LEAST_GLOBAL_CONNECTIONS, /*< all connections established by MaxScale */ - LEAST_ROUTER_CONNECTIONS, /*< connections established by this router */ - LEAST_BEHIND_MASTER, - LEAST_CURRENT_OPERATIONS, - DEFAULT_CRITERIA=LEAST_CURRENT_OPERATIONS, - LAST_CRITERIA /*< not used except for an index */ -} select_criteria_t; - - /** default values for rwsplit configuration parameters */ #define CONFIG_MAX_SLAVE_CONN 1 #define CONFIG_MAX_SLAVE_RLAG -1 /*< not used */ @@ -229,11 +212,9 @@ typedef struct backend_ref_st { } backend_ref_t; -typedef struct rwsplit_config_st { +typedef struct dbshard_config_st { int rw_max_slave_conn_percent; int rw_max_slave_conn_count; - select_criteria_t rw_slave_select_criteria; - int rw_max_slave_replication_lag; target_t rw_use_sql_variables_in; } rwsplit_config_t; diff --git a/server/modules/routing/dbshard/dbshard.c b/server/modules/routing/dbshard/dbshard.c index da2dbda45..16e418403 100644 --- a/server/modules/routing/dbshard/dbshard.c +++ b/server/modules/routing/dbshard/dbshard.c @@ -98,7 +98,7 @@ static void print_error_packet(ROUTER_CLIENT_SES* rses, GWBUF* buf, DCB* dcb); static int router_get_servercount(ROUTER_INSTANCE* router); static backend_ref_t* get_bref_from_dcb(ROUTER_CLIENT_SES* rses, DCB* dcb); -static route_target_t get_route_target ( +static route_target_t get_shard_route_target ( skygw_query_type_t qtype, bool trx_active, target_t use_sql_variables_in, @@ -146,12 +146,10 @@ static bool connect_backend_servers( SESSION* session, ROUTER_INSTANCE* router); -static bool get_dcb( +static bool get_shard_dcb( DCB** dcb, ROUTER_CLIENT_SES* rses, - backend_type_t btype, - char* name, - int max_rlag); + char* name); #if 0 static void rwsplit_process_router_options( ROUTER_INSTANCE* router, @@ -175,9 +173,6 @@ static bool rses_begin_locked_router_action( static void rses_end_locked_router_action( ROUTER_CLIENT_SES* rses); -static int rses_get_max_replication_lag( - ROUTER_CLIENT_SES* rses); - static void mysql_sescmd_done( mysql_sescmd_t* sescmd); @@ -320,10 +315,12 @@ static void* hfree(void* fval) /** - * Updates the hashtable with the database names and where to find them, adding new and removing obsolete pairs. + * Updates the hashtable with the database names and where to find them, adding + * new and removing obsolete pairs. * @param backends Backends to query for database names * @param hashtable Hashtable to use - * @return True if all database and server names were successfully retrieved otherwise false + * @return True if all database and server names were successfully retrieved + * otherwise false */ bool update_dbnames_hash(BACKEND** backends, HASHTABLE* hashtable) { @@ -336,167 +333,198 @@ bool update_dbnames_hash(BACKEND** backends, HASHTABLE* hashtable) MYSQL_ROW row; int i, rc, numfields; - for(i = 0;backends[i] && rval;i++){ + for(i = 0;backends[i] && rval;i++) + { + handle = mysql_init(NULL); - handle = mysql_init(NULL); - - if(handle == NULL){ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error: Failed to initialize MySQL handle."))); - continue; - } - - rc = 0; - rc |= mysql_options(handle, MYSQL_OPT_CONNECT_TIMEOUT, (void *)&connect_timeout); - rc |= mysql_options(handle, MYSQL_OPT_READ_TIMEOUT, (void *)&read_timeout); - if(rc != 0){ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error: Failed to set MySQL connection options."))); - mysql_close(handle); - rval = false; - continue; - } + if(handle == NULL){ + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to initialize MySQL handle."))); + continue; + } + rc = 0; + rc |= mysql_options(handle, + MYSQL_OPT_CONNECT_TIMEOUT, + (void *)&connect_timeout); + rc |= mysql_options(handle, + MYSQL_OPT_READ_TIMEOUT, + (void *)&read_timeout); + + if(rc != 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to set MySQL connection options."))); + mysql_close(handle); + rval = false; + continue; + } server = backends[i]->backend_server; ss_dassert(server != NULL); if (mysql_real_connect(handle, - server->name, - server->monuser, - server->monpw, - NULL, - server->port, - NULL, - 0) == NULL) - { + server->name, + server->monuser, + server->monpw, + NULL, + server->port, + NULL, + 0) == NULL) + { LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error: Failed to connect to backend server '%s'.",server->name))); + LOGFILE_ERROR, + "Error: Failed to connect to backend " + "server '%s'.", + server->name))); rval = false; goto cleanup; - } - + } /** * The server was successfully connected to, proceed to query for database names */ - - if((result = mysql_list_dbs(handle,NULL)) == NULL){ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, - "Error: Failed to execute query in backend server '%s'.",server->name))); + if((result = mysql_list_dbs(handle,NULL)) == NULL) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to execute query in backend " + "server '%s'.", + server->name))); goto cleanup; } - numfields = mysql_num_fields(result); - if(numfields < 1){ - LOGIF(LT, (skygw_log_write_flush(LOGFILE_TRACE, - "Backend '%s' has no databases.",server->name))); + if(numfields < 1) + { + LOGIF(LT, (skygw_log_write_flush( + LOGFILE_TRACE, + "Backend '%s' has no databases.", + server->name))); goto cleanup; } - /** * Walk through the list of databases in this backend * and insert them into the hashtable. If the value is already in the hashtable * but the backend isn't in the list of backends it is replaced with the first found backend. */ - while((row = mysql_fetch_row(result))) + { + unsigned long *lengths; + char *dbnm = NULL,*servnm = NULL; + + lengths = mysql_fetch_lengths(result); + if(strncmp(row[0],"information_schema",lengths[0]) == 0){ + continue; + } + dbnm = (char*)calloc(lengths[0] + 1,sizeof(char)); + memcpy(dbnm,row[0],lengths[0]); + servnm = strdup(server->unique_name); + + if(hashtable_add(hashtable,dbnm,servnm) == 0) { - - unsigned long *lengths; - char *dbnm = NULL,*servnm = NULL; - - lengths = mysql_fetch_lengths(result); - if(strncmp(row[0],"information_schema",lengths[0]) == 0){ - continue; - } - dbnm = (char*)calloc(lengths[0] + 1,sizeof(char)); - memcpy(dbnm,row[0],lengths[0]); - servnm = strdup(server->unique_name); - - if(hashtable_add(hashtable,dbnm,servnm) == 0){ - #ifdef SHARD_UPDATES - { + { char* old_backend = (char*)hashtable_fetch(hashtable,dbnm); int j; bool is_alive = false; - for(j = 0;backends[j];j++){ + for(j = 0;backends[j];j++) + { /** - * See if the old backend is still alive. If not then update + * See if the old backend is still + * alive. If not then update * the hashtable with the current backend's name. */ if(strcmp(server->unique_name,old_backend) == 0 && - SERVER_IS_RUNNING(server)){ + SERVER_IS_RUNNING(server)) + { is_alive = true; } } - - if(!is_alive){ + + if(!is_alive) + { hashtable_delete(hashtable,dbnm); - if(hashtable_add(hashtable,dbnm,servnm)){ - LOGIF(LT, (skygw_log_write_flush(LOGFILE_TRACE, - "Updated the backend of database '%s' to '%s'.",dbnm,servnm))); - }else{ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, - "Error: Failed to insert values into hashtable."))); + + if(hashtable_add(hashtable,dbnm,servnm)) + { + LOGIF(LT, (skygw_log_write_flush( + LOGFILE_TRACE, + "Updated the backend of database '%s' to '%s'.", + dbnm, + servnm))); + } + else + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to insert values into hashtable."))); } } goto cleanup; - } -#endif - /*Check if the failure was due to a duplicate value*/ - if(hashtable_fetch(hashtable,dbnm) == NULL){ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, - "Error: Failed to insert values into hashtable."))); - }else{ - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, - "Error: Duplicate value found."))); - } - rval = false; - free(dbnm); - free(servnm); } - } - - cleanup: - - if(result){ +#endif /*< SHARD_UPDATES */ + /*Check if the failure was due to a duplicate value*/ + if(hashtable_fetch(hashtable,dbnm) == NULL) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Failed to insert values into hashtable."))); + } + else + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Duplicate value found."))); + } + rval = false; + free(dbnm); + free(servnm); + } /*< hashtable_add failed */ + } /*< while */ +cleanup: + if(result) + { mysql_free_result(result); } result = NULL; mysql_close(handle); - } + } /*< for */ return rval; } /** - * Allocates a new hashtable and inserts database names and where to find them into it. + * Allocates a new hashtable and inserts database names and where to find them + * into it. * @param backends Backends to query for database names * @return Pointer to the newly allocated hashtable or NULL if an error occurred */ void* dbnames_hash_init(BACKEND** backends) { - HASHTABLE* htbl = hashtable_alloc(32,hashkeyfun,hashcmpfun); + HASHTABLE* htbl = hashtable_alloc(512,hashkeyfun,hashcmpfun); if(htbl == NULL) - { - LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Error: Hashtable allocation failed."))); - return NULL; - } - - - - /**Update the new hashtable with the key-value pairs*/ - if(!update_dbnames_hash(backends,htbl)){ - //LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Errors encountered while querying databases."))); - //hashtable_free(htbl); - //return NULL; + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error: Hashtable allocation failed."))); + return NULL; + } + /**Update the new hashtable with the key-value pairs*/ + if(!update_dbnames_hash(backends,htbl)) + { + /* + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,"Errors encountered while querying databases."))); + hashtable_free(htbl); + return NULL; + */ + hashtable_free(htbl); + htbl = NULL; } - return htbl; - } bool add_shard_info(GWBUF* buffer, char* target) @@ -516,10 +544,12 @@ char* get_shard_target_name(ROUTER_INSTANCE* router, GWBUF* buffer){ } dbnms = skygw_get_database_names(buffer,&sz); - if(sz > 0){ - - for(i = 0; i < sz; i++){ - if((rval = (char*)hashtable_fetch(ht,dbnms[i]))){ + if(sz > 0) + { + for(i = 0; i < sz; i++) + { + if((rval = (char*)hashtable_fetch(ht,dbnms[i]))) + { break; } } @@ -594,51 +624,9 @@ static void refreshInstance( if (paramtype == COUNT_TYPE) { - if (strncmp(param->name, "max_slave_connections", MAX_PARAM_LEN) == 0) - { - int val; - bool succp; - - router->dbshard_config.rw_max_slave_conn_percent = 0; - - succp = config_get_valint(&val, param, NULL, paramtype); - - if (succp) - { - router->dbshard_config.rw_max_slave_conn_count = val; - } - } - else if (strncmp(param->name, - "max_slave_replication_lag", - MAX_PARAM_LEN) == 0) - { - int val; - bool succp; - - succp = config_get_valint(&val, param, NULL, paramtype); - - if (succp) - { - router->dbshard_config.rw_max_slave_replication_lag = val; - } - } } else if (paramtype == PERCENT_TYPE) { - if (strncmp(param->name, "max_slave_connections", MAX_PARAM_LEN) == 0) - { - int val; - bool succp; - - router->dbshard_config.rw_max_slave_conn_count = 0; - - succp = config_get_valint(&val, param, NULL, paramtype); - - if (succp) - { - router->dbshard_config.rw_max_slave_conn_percent = val; - } - } } else if (paramtype == SQLVAR_TARGET_TYPE) { @@ -760,13 +748,7 @@ createInstance(SERVICE *service, char **options) while (server != NULL) { if ((router->servers[nservers] = malloc(sizeof(BACKEND))) == NULL) { - /** clean up */ - for (i = 0; i < nservers; i++) { - free(router->servers[i]); - } - free(router->servers); - free(router); - return NULL; + goto clean_up; } router->servers[nservers]->backend_server = server; router->servers[nservers]->backend_conn_count = 0; @@ -791,11 +773,16 @@ createInstance(SERVICE *service, char **options) * is used if any. */ router->dbshard_version = service->svc_config_version; - //refreshInstance(router, NULL); + /** refreshInstance(router, NULL); */ /** * Get hashtable which includes dbname,backend pairs */ - router->dbnames_hash = (HASHTABLE*)dbnames_hash_init(router->servers); + router->dbnames_hash = (HASHTABLE*)dbnames_hash_init(router->servers); + + if (router->dbnames_hash == NULL) + { + goto clean_up; + } /** * We have completed the creation of the router data, so now * insert this router into the linked list of routers @@ -805,8 +792,20 @@ createInstance(SERVICE *service, char **options) router->next = instances; instances = router; spinlock_release(&instlock); - - return (ROUTER *)router; + goto retblock; + +clean_up: + /** clean up */ + for (i = 0; i < nservers; i++) + { + free(router->servers[i]); + } + free(router->servers); + free(router); + router = NULL; + /** Fallthrough */ +retblock: + return (ROUTER *)router; } /** @@ -829,8 +828,13 @@ static void* newSession( bool succp; int router_nservers = 0; /*< # of servers in total */ int i; - //const int min_nservers = 1; /*< hard-coded for now */ - +#if 0 + /** + * It could be possibe to accept new session if some of the servers are + * not reachable + */ + const int min_nservers = 1; /*< hard-coded for now */ +#endif client_rses = (ROUTER_CLIENT_SES *)calloc(1, sizeof(ROUTER_CLIENT_SES)); if (client_rses == NULL) @@ -939,6 +943,9 @@ static void* newSession( client_rses = NULL; goto return_rses; } + /** + * Connect to all backend servers + */ succp = connect_backend_servers(backend_ref, router_nservers, session, @@ -946,7 +953,7 @@ static void* newSession( rses_end_locked_router_action(client_rses); - /** + /** * Master and at least slaves must be found */ if (!succp) { @@ -959,7 +966,6 @@ static void* newSession( client_rses->rses_capabilities = RCAP_TYPE_STMT_INPUT; client_rses->rses_backend_ref = backend_ref; client_rses->rses_nbackends = router_nservers; /*< # of backend servers */ - client_rses->rses_master_ref = get_root_master_bref(client_rses); router->stats.n_sessions += 1; /** @@ -1147,210 +1153,45 @@ static void freeSession( * * @return True if proper DCB was found, false otherwise. */ -static bool get_dcb( +static bool get_shard_dcb( DCB** p_dcb, ROUTER_CLIENT_SES* rses, - backend_type_t btype, - char* name, - int max_rlag) + char* name) { backend_ref_t* backend_ref; - backend_ref_t* master_bref; int i; bool succp = false; - BACKEND* master_host; CHK_CLIENT_RSES(rses); ss_dassert(p_dcb != NULL && *(p_dcb) == NULL); - if (p_dcb == NULL) + if (p_dcb == NULL || name == NULL) { goto return_succp; } backend_ref = rses->rses_backend_ref; - - /** 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) + + for (i=0; irses_nbackends; i++) { - goto return_succp; - } -#if defined(SS_DEBUG) - /** master_host is just for additional checking */ - master_host = get_root_master(backend_ref, rses->rses_nbackends); - 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 */ - { - 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; - /** - * To become chosen: - * backend must be in use, name must match, - * root master node must be found, - * backend's role must be either slave, relay - * server, or master. - */ - if (BREF_IS_IN_USE((&backend_ref[i])) && - (strncasecmp( - name, - b->backend_server->unique_name, - PATH_MAX) == 0) && - master_bref->bref_backend != NULL && - (SERVER_IS_SLAVE(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; - ss_dassert(backend_ref[i].bref_dcb->state != DCB_STATE_ZOMBIE); - break; - } - } - if (succp) + BACKEND* b = backend_ref[i].bref_backend; + /** + * To become chosen: + * backend must be in use, name must match, and + * the backend state must be RUNNING + */ + if (BREF_IS_IN_USE((&backend_ref[i])) && + (strncasecmp( + name, + b->backend_server->unique_name, + PATH_MAX) == 0) && + SERVER_IS_RUNNING(b->backend_server)) { + *p_dcb = backend_ref[i].bref_dcb; + succp = true; + ss_dassert(backend_ref[i].bref_dcb->state != DCB_STATE_ZOMBIE); goto return_succp; } - else - { - btype = BE_SLAVE; - } } - - if (btype == BE_SLAVE) - { - backend_ref_t* candidate_bref = NULL; - - for (i=0; irses_nbackends; i++) - { - BACKEND* b = (&backend_ref[i])->bref_backend; - /** - * Unused backend or backend which is not master nor - * slave can't be used - */ - if (!BREF_IS_IN_USE(&backend_ref[i]) || - (!SERVER_IS_MASTER(b->backend_server) && - !SERVER_IS_SLAVE(b->backend_server))) - { - continue; - } - /** - * If there are no candidates yet accept both master or - * slave. - */ - else if (candidate_bref == NULL) - { - /** - * Ensure that master has not changed dunring - * session and abort if it has. - */ - if (SERVER_IS_MASTER(b->backend_server) && - &backend_ref[i] == master_bref) - { - /** found master */ - candidate_bref = &backend_ref[i]; - succp = true; - } - /** - * Ensure that max replication lag is not set - * or that candidate's lag doesn't exceed the - * maximum allowed replication lag. - */ - else if (max_rlag == MAX_RLAG_UNDEFINED || - (b->backend_server->rlag != MAX_RLAG_NOT_AVAILABLE && - b->backend_server->rlag <= max_rlag)) - { - /** found slave */ - candidate_bref = &backend_ref[i]; - succp = true; - } - } - /** - * If candidate is master, any slave which doesn't break - * replication lag limits replaces it. - */ - else if (SERVER_IS_MASTER(candidate_bref->bref_backend->backend_server) && - 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))) - { - /** found slave */ - candidate_bref = &backend_ref[i]; - succp = true; - } - /** - * When candidate exists, compare it against the current - * backend and update assign it to new candidate if - * necessary. - */ - else if (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))) - { -#if 0 - candidate_bref = check_candidate_bref( - candidate_bref, - &backend_ref[i], - rses->rses_config.rw_slave_select_criteria); -#endif - } - else - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Server %s:%d is too much behind the " - "master, %d s. and can't be chosen.", - b->backend_server->name, - b->backend_server->port, - b->backend_server->rlag))); - } - } /*< for */ - /** Assign selected DCB's pointer value */ - if (candidate_bref != NULL) - { - *p_dcb = candidate_bref->bref_dcb; - } - - goto return_succp; - } /*< if (btype == BE_SLAVE) */ - /** - * If target was originally master only then the execution jumps - * directly here. - */ - if (btype == BE_MASTER) - { - if (BREF_IS_IN_USE(master_bref) && - SERVER_IS_MASTER(master_bref->bref_backend->backend_server)) - { - *p_dcb = master_bref->bref_dcb; - succp = true; - /** if bref is in use DCB should not be closed */ - ss_dassert(master_bref->bref_dcb->state != DCB_STATE_ZOMBIE); - } - else - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Server at %s:%d should be master but " - "is %s instead and can't be chosen to master.", - master_bref->bref_backend->backend_server->name, - master_bref->bref_backend->backend_server->port, - STRSRVSTATUS(master_bref->bref_backend->backend_server)))); - succp = false; - } - } return_succp: return succp; @@ -1407,7 +1248,7 @@ static backend_ref_t* check_candidate_bref( * @return bitfield including the routing target, or the target server name * if the query would otherwise be routed to slave. */ -static route_target_t get_route_target ( +static route_target_t get_shard_route_target ( skygw_query_type_t qtype, bool trx_active, /*< !!! turha ? */ target_t use_sql_variables_in, /*< 'master' == single tässä tapauksessa */ @@ -1433,7 +1274,7 @@ static route_target_t get_route_target ( } else { - target = TARGET_SINGLE; + target = TARGET_NAMED_SERVER; } #if defined(SS_DEBUG) LOGIF(LT, (skygw_log_write( @@ -1747,14 +1588,12 @@ static int routeQuery( mysql_server_cmd_t packet_type; uint8_t* packet; int ret = 0; - DCB* master_dcb = NULL; DCB* target_dcb = NULL; ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; ROUTER_CLIENT_SES* router_cli_ses = (ROUTER_CLIENT_SES *)router_session; bool rses_is_closed = false; route_target_t route_target; bool succp = false; - backend_type_t btype; /*< target backend type */ char* tname = NULL; CHK_CLIENT_RSES(router_cli_ses); @@ -1768,8 +1607,8 @@ static int routeQuery( packet = GWBUF_DATA(querybuf); packet_type = packet[4]; - master_dcb = router_cli_ses->rses_master_ref->bref_dcb; - if (rses_is_closed) + + if (rses_is_closed) { /** * MYSQL_COM_QUIT may have sent by client and as a part of backend @@ -1798,7 +1637,7 @@ static int routeQuery( { querybuf = gwbuf_make_contiguous(querybuf); } - + switch(packet_type) { case MYSQL_COM_QUIT: /*< 1 QUIT will close all sessions */ case MYSQL_COM_INIT_DB: /*< 2 DDL must go to the master */ @@ -1922,6 +1761,7 @@ static int routeQuery( /** * Find out whether the query should be routed to single server or to * all of them. + */ /** * Find out where to route the query. Result may not be clear; it is * possible to have a hint for routing to a named server which can @@ -1940,22 +1780,21 @@ static int routeQuery( * eventually to master */ - /** * Added for simple sharding, using hints for testing. */ - if((tname = get_shard_target_name(inst,querybuf)) != NULL && - add_shard_info(querybuf,tname)){ - - route_target = TARGET_NAMED_SERVER; - }else{ - - route_target = get_route_target(qtype, - router_cli_ses->rses_transaction_active, + if((tname = get_shard_target_name(inst,querybuf)) != NULL && + add_shard_info(querybuf,tname)) + { + route_target = TARGET_NAMED_SERVER; + } + else + { + route_target = get_shard_route_target(qtype, + router_cli_ses->rses_transaction_active, router_cli_ses->rses_config.rw_use_sql_variables_in, - querybuf->hint); - } - + querybuf->hint); + } if (TARGET_IS_ALL(route_target)) { @@ -1988,172 +1827,24 @@ static int routeQuery( goto retblock; } /** - * There is a hint which either names the target backend or - * hint which sets maximum allowed replication lag for the - * backend. + * Query is routed to one of the backends */ - if (TARGET_IS_NAMED_SERVER(route_target) || - TARGET_IS_RLAG_MAX(route_target)) - { - HINT* hint; - char* named_server = NULL; - - hint = querybuf->hint; - - while (hint != NULL) - { - if (hint->type == HINT_ROUTE_TO_NAMED_SERVER) - { - /** - * Set the name of searched - * backend server. - */ - named_server = hint->data; - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Hint: route to server " - "'%s'", - named_server))); - } - else if (hint->type == HINT_PARAMETER && - (strncasecmp((char *)hint->data, - "max_slave_replication_lag", - strlen("max_slave_replication_lag")) == 0)) - { - int val = (int) strtol((char *)hint->value, - (char **)NULL, 10); - - if (val != 0 || errno == 0) - { - /** - * Set max. acceptable - * replication lag - * value for backend srv - */ - rlag_max = val; - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Hint: " - "max_slave_replication_lag=%d", - rlag_max))); - } - } - hint = hint->next; - } /*< while */ - - if (rlag_max == MAX_RLAG_UNDEFINED) /*< no rlag max hint, use config */ - { - rlag_max = rses_get_max_replication_lag(router_cli_ses); - } - btype = BE_UNDEFINED; /*< target may be master or slave */ + if (TARGET_IS_NAMED_SERVER(route_target)) + { /** * Search backend server by name or replication lag. * If it fails, then try to find valid slave or master. */ - succp = get_dcb(&target_dcb, - router_cli_ses, - btype, - named_server, - rlag_max); + succp = get_shard_dcb(&target_dcb, router_cli_ses, tname); + if (!succp) { - if (TARGET_IS_NAMED_SERVER(route_target)) - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Was supposed to route to named server " - "%s but couldn't find the server in a " - "suitable state.", - named_server))); - } - else if (TARGET_IS_RLAG_MAX(route_target)) - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Was supposed to route to server with " - "replication lag at most %d but couldn't " - "find such a slave.", - rlag_max))); - } - } - } - else if (TARGET_IS_SLAVE(route_target)) - { - btype = BE_SLAVE; - - if (rlag_max == MAX_RLAG_UNDEFINED) /*< no rlag max hint, use config */ - { - rlag_max = rses_get_max_replication_lag(router_cli_ses); - } - /** - * Search suitable backend server, get DCB in target_dcb - */ - succp = get_dcb(&target_dcb, - router_cli_ses, - BE_SLAVE, - NULL, - rlag_max); - if (succp) - { -#if defined(SS_DEBUG) - LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, - "Found DCB for slave."))); - ss_dassert(get_bref_from_dcb(router_cli_ses, target_dcb) != - router_cli_ses->rses_master_ref); - ss_dassert(get_root_master_bref(router_cli_ses) == - router_cli_ses->rses_master_ref); -#endif - atomic_add(&inst->stats.n_slave, 1); - } - else - { - LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, - "Was supposed to route to slave" - "but finding suitable one " - "failed."))); - } - } - else if (TARGET_IS_MASTER(route_target)) - { - DCB* curr_master_dcb = NULL; - - succp = get_dcb(&curr_master_dcb, - router_cli_ses, - BE_MASTER, - NULL, - MAX_RLAG_UNDEFINED); - - if (succp && master_dcb == curr_master_dcb) - { - 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 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."))); - } - /** - * Master has changed. Return with error indicator. - */ - rses_end_locked_router_action(router_cli_ses); - succp = false; - ret = 0; - goto retblock; + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Was supposed to route to named server " + "%s but couldn't find the server in a " + "suitable state.", + tname))); } } @@ -2167,9 +1858,7 @@ static int routeQuery( LOGIF(LT, (skygw_log_write( LOGFILE_TRACE, - "Route query to %s\t%s:%d <", - (SERVER_IS_MASTER(bref->bref_backend->backend_server) ? - "master" : "slave"), + "Route query to \t%s:%d <", bref->bref_backend->backend_server->name, bref->bref_backend->backend_server->port))); /** @@ -2707,14 +2396,17 @@ static bool connect_backend_servers( ROUTER_INSTANCE* router) { bool succp = true; - //bool is_synced_master; - bool master_connected = true; + /* + bool is_synced_master; + bool master_connected = true; + */ int servers_found = 0; int servers_connected = 0; int slaves_connected = 0; int i,max_nservers = router_nservers; - select_criteria_t select_criteria = LEAST_GLOBAL_CONNECTIONS; - + /* + select_criteria_t select_criteria = LEAST_GLOBAL_CONNECTIONS; + */ #if 0 if (router->bitvalue != 0) /*< 'synced' is the only bitvalue in rwsplit */ { @@ -2745,35 +2437,26 @@ static bool connect_backend_servers( if (LOG_IS_ENABLED(LOGFILE_TRACE)) { - if (select_criteria == LEAST_GLOBAL_CONNECTIONS || - select_criteria == LEAST_ROUTER_CONNECTIONS || - select_criteria == LEAST_BEHIND_MASTER || - select_criteria == LEAST_CURRENT_OPERATIONS) - { - LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, - "Servers and connection counts:"))); + LOGIF(LT, (skygw_log_write(LOGFILE_TRACE, + "Servers and connection counts:"))); - for (i=0; ibackend_conn_count, - b->backend_server->stats.n_current, - b->backend_server->name, - b->backend_server->port, - STRSRVSTATUS(b->backend_server)))); - } - } - } /*< log only */ + for (i=0; ibackend_conn_count, + b->backend_server->stats.n_current, + b->backend_server->name, + b->backend_server->port, + STRSRVSTATUS(b->backend_server)))); + } + } /*< log only */ /** * Choose at least onr server from the list. */ - for (i=0; - ibackend_server->name, b->backend_server->port))); /* handle connect error */ + break; } } } @@ -2864,56 +2549,50 @@ static bool connect_backend_servers( /** * Successful cases */ - if (servers_connected > 0) + if (servers_connected == router_nservers) { - if (servers_connected == 0) + succp = true; + + if (LOG_IS_ENABLED(LT)) { - /** Failure case */ - succp = false; - } - else - { - succp = true; - - if (LOG_IS_ENABLED(LT)) + for (i=0; ibackend_server), - b->backend_server->name, - b->backend_server->port))); - } - } /* for */ + BACKEND* b = backend_ref[i].bref_backend; + + if (BREF_IS_IN_USE((&backend_ref[i]))) + { + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Connected %s in \t%s:%d", + STRSRVSTATUS(b->backend_server), + b->backend_server->name, + b->backend_server->port))); + } + } /* for */ + } + } + else + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "Warning : Couldn't connect to all available " + "servers. Session can't be created."))); + + /** Clean up connections */ + for (i=0; ibackend_conn_count > 0); + + /** disconnect opened connections */ + dcb_close(backend_ref[i].bref_dcb); + bref_clear_state(&backend_ref[i], BREF_IN_USE); + /** Decrease backend's connection counter. */ + atomic_add(&backend_ref[i].bref_backend->backend_conn_count, -1); } } - - if (servers_connected < router_nservers) - { - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "Warning : Couldn't connect to all available " - "servers. Routing to %d out of %d only.", - servers_found, - router_nservers))); - - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "* Warning : Couldn't connect to all available " - "servers. Routing to %d out of %d only.", - servers_found, - router_nservers))); - } - } - - //return_succp: - + } return succp; } @@ -4094,25 +3773,6 @@ static int rses_get_max_slavecount( } #endif -static int rses_get_max_replication_lag( - ROUTER_CLIENT_SES* rses) -{ - int conf_max_rlag; - - CHK_CLIENT_RSES(rses); - - /** if there is no configured value, then longest possible int is used */ - if (rses->rses_config.rw_max_slave_replication_lag > 0) - { - conf_max_rlag = rses->rses_config.rw_max_slave_replication_lag; - } - else - { - conf_max_rlag = ~(1<<31); - } - - return conf_max_rlag; -} /** * Finds out if there is a backend reference pointing at the DCB given as @@ -4364,7 +4024,6 @@ static void dbshard_process_router_options( { int i; char* value; - select_criteria_t c; for (i = 0; options[i]; i++) { @@ -4380,6 +4039,7 @@ static void dbshard_process_router_options( { *value = 0; value++; +#if 0 if (strcmp(options[i], "slave_selection_criteria") == 0) { c = GET_SELECT_CRITERIA(value); @@ -4406,6 +4066,7 @@ static void dbshard_process_router_options( router->dbshard_config.rw_slave_select_criteria = c; } } +#endif } } /*< for */ } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c21ecf26b..796280542 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -3220,8 +3220,6 @@ static bool select_connect_backend_servers( atomic_add(&backend_ref[i].bref_backend->backend_conn_count, -1); } } - master_connected = false; - slaves_connected = 0; } return_succp: