From 96f76a1f2eba4d1d368c4ba430adc3095113fd88 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 25 Sep 2015 21:23:09 +0300 Subject: [PATCH] Changed the way max_sescmd_history works and combined disable_sescmd_history and disable_slave_recovery. Before these changes when max_sescmd_history was used the session was closed when the limit was exceeded. With this change, when the limit is exceeded the recovery of slaves and the session command history are both disabled. This will allow the sessions to continue while still keeping the old functionality of limited salve replacement. The disable_sescmd_history and disable_slave_recovery parameters were combined so that disabling the session command history will also disable slave recovery. This way no harm can be done with disable_sescmd_history. --- Documentation/Routers/ReadWriteSplit.md | 14 +++---- server/modules/include/readwritesplit.h | 22 +++++----- .../routing/readwritesplit/readwritesplit.c | 40 ++++++++----------- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/Documentation/Routers/ReadWriteSplit.md b/Documentation/Routers/ReadWriteSplit.md index f75a5873b..ec89db65f 100644 --- a/Documentation/Routers/ReadWriteSplit.md +++ b/Documentation/Routers/ReadWriteSplit.md @@ -73,9 +73,12 @@ When value all is used, queries reading session variables can be routed to any a In above-mentioned case the user-defined variable would only be updated in the master where query would be routed due to `INSERT` statement. -**`max_sescmd_history`** sets a limit on how many session commands each session can execute before the connection is closed. The default is an unlimited number of session commands. +**`max_sescmd_history`** sets a limit on how many session commands each session can execute before the session command history is disabled. The default is an unlimited number of session commands. - max_sescmd_history=1500 +``` +# Set a limit on the session command history +max_sescmd_history=1500 +``` When a limitation is set, it effectively creates a cap on the session's memory consumption. This might be useful if connection pooling is used and the sessions use large amounts of session commands. @@ -86,13 +89,6 @@ When a limitation is set, it effectively creates a cap on the session's memory c disable_sescmd_history=true ``` -**`disable_slave_recovery`** disables the recovery and replacement of slave servers. If this option is enabled and a connection to a slave server in use is lost, no replacement slave will be taken. This allows the safe use of session state modifying statements when the session command history is disabled. This is mostly intended to be used with the `disable_sescmd_history` option enabled. - -``` -# Disable the session command history -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. ``` diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index c78cde5b4..9f9866e5d 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -242,19 +242,17 @@ typedef struct backend_ref_st { #endif } backend_ref_t; - -typedef struct rwsplit_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; - int rw_max_sescmd_history_size; - bool disable_sescmd_hist; - bool disable_slave_recovery; - bool master_reads; /*< Use master for reads */ +typedef struct rwsplit_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; + int rw_max_sescmd_history_size; + bool rw_disable_sescmd_hist; + bool rw_master_reads; /*< Use master for reads */ } rwsplit_config_t; - #if defined(PREP_STMT_CACHING) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 60c724958..15ad5ca42 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -702,7 +702,7 @@ createInstance(SERVICE *service, char **options) } /** These options cancel each other out */ - if(router->rwsplit_config.disable_sescmd_hist && router->rwsplit_config.rw_max_sescmd_history_size > 0) + if(router->rwsplit_config.rw_disable_sescmd_hist && router->rwsplit_config.rw_max_sescmd_history_size > 0) { router->rwsplit_config.rw_max_sescmd_history_size = 0; } @@ -1246,7 +1246,7 @@ static bool get_dcb( (max_rlag == MAX_RLAG_UNDEFINED || (b->backend_server->rlag != MAX_RLAG_NOT_AVAILABLE && b->backend_server->rlag <= max_rlag)) && - !rses->rses_config.master_reads) + !rses->rses_config.rw_master_reads) { /** found slave */ candidate_bref = &backend_ref[i]; @@ -2897,7 +2897,7 @@ static void clientReply ( bool rconn = false; writebuf = sescmd_cursor_process_replies(writebuf, bref, &rconn); - if(rconn && !router_inst->rwsplit_config.disable_slave_recovery) + if(rconn && !router_inst->rwsplit_config.rw_disable_sescmd_hist) { select_connect_backend_servers(&router_cli_ses->rses_master_ref, router_cli_ses->rses_backend_ref, @@ -4541,21 +4541,17 @@ static bool route_session_write( goto return_succp; } - if(router_cli_ses->rses_config.rw_max_sescmd_history_size > 0 && - router_cli_ses->rses_nsescmd >= router_cli_ses->rses_config.rw_max_sescmd_history_size) - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Router session exceeded session command history limit. " - "Closing router session. <"))); - gwbuf_free(querybuf); - rses_end_locked_router_action(router_cli_ses); - router_cli_ses->client_dcb->func.hangup(router_cli_ses->client_dcb); - - goto return_succp; - } + if (router_cli_ses->rses_config.rw_max_sescmd_history_size > 0 && + router_cli_ses->rses_nsescmd >= router_cli_ses->rses_config.rw_max_sescmd_history_size) + { + skygw_log_write(LE, "Warning: Router session exceeded session command history limit. " + "Slave recovery is disabled and only slave servers with consistent session state are used " + "for the duration of the session."); + router_cli_ses->rses_config.rw_disable_sescmd_hist = true; + router_cli_ses->rses_config.rw_max_sescmd_history_size = 0; + } - if(router_cli_ses->rses_config.disable_sescmd_hist) + if(router_cli_ses->rses_config.rw_disable_sescmd_hist) { rses_property_t *prop, *tmp; backend_ref_t* bref; @@ -4784,15 +4780,11 @@ static void rwsplit_process_router_options( } else if(strcmp(options[i],"disable_sescmd_history") == 0) { - router->rwsplit_config.disable_sescmd_hist = config_truth_value(value); - } - else if(strcmp(options[i],"disable_slave_recovery") == 0) - { - router->rwsplit_config.disable_slave_recovery = config_truth_value(value); + router->rwsplit_config.rw_disable_sescmd_hist = config_truth_value(value); } else if(strcmp(options[i],"master_accept_reads") == 0) { - router->rwsplit_config.master_reads = config_truth_value(value); + router->rwsplit_config.rw_master_reads = config_truth_value(value); } } } /*< for */ @@ -5039,7 +5031,7 @@ static bool handle_error_new_connection( * Try to get replacement slave or at least the minimum * number of slave connections for router session. */ - if(inst->rwsplit_config.disable_slave_recovery) + if(inst->rwsplit_config.rw_disable_sescmd_hist) { succp = have_enough_servers(&myrses,1,router_nservers,inst) ? true : false; }