hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove spinlocks from readwritesplit

Browse Source 
Spinlocks are no longer necessary as the threading model only allows a
specific thread to access the router session.
This commit is contained in:
Markus Mäkelä
2017-02-27 09:54:57 +02:00
parent d834c1bbdc
commit 677e8aaabf
6 changed files with 18 additions and 197 deletions
Download Patch File Download Diff File Expand all files Collapse all files

132
server/modules/routing/readwritesplit/readwritesplit.c
View File

@ -250,7 +250,6 @@ static MXS_ROUTER *createInstance(SERVICE *service, char **options)
return NULL;
}
router->service = service;
spinlock_init(&router->lock);
/*
* Until we know otherwise assume we have some available slaves.
@ -331,7 +330,6 @@ static MXS_ROUTER_SESSION *newSession(MXS_ROUTER *router_inst, MXS_SESSION *sess
client_rses->client_dcb = session->client_dcb;
client_rses->have_tmp_tables = false;
client_rses->forced_node = NULL;
spinlock_init(&client_rses->rses_lock);
memcpy(&client_rses->rses_config, &router->rwsplit_config, sizeof(client_rses->rses_config));
int router_nservers = router->service->n_dbref;
@ -409,7 +407,7 @@ static void closeSession(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_sessio
ROUTER_CLIENT_SES *router_cli_ses = (ROUTER_CLIENT_SES *)router_session;
CHK_CLIENT_RSES(router_cli_ses);
if (!router_cli_ses->rses_closed && rses_begin_locked_router_action(router_cli_ses))
if (!router_cli_ses->rses_closed)
{
/**
* Mark router session as closed. @c rses_closed is checked at the start
@ -465,8 +463,6 @@ static void closeSession(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_sessio
}
}
}
rses_end_locked_router_action(router_cli_ses);
}
}
@ -670,31 +666,22 @@ static void clientReply(MXS_ROUTER *instance,
GWBUF *writebuf,
DCB *backend_dcb)
{
DCB *client_dcb;
ROUTER_INSTANCE *router_inst;
ROUTER_CLIENT_SES *router_cli_ses;
sescmd_cursor_t *scur = NULL;
backend_ref_t *bref;
ROUTER_CLIENT_SES *router_cli_ses = (ROUTER_CLIENT_SES *)router_session;
ROUTER_INSTANCE *router_inst = (ROUTER_INSTANCE *)instance;
DCB *client_dcb = backend_dcb->session->client_dcb;
router_cli_ses = (ROUTER_CLIENT_SES *)router_session;
router_inst = (ROUTER_INSTANCE *)instance;
CHK_CLIENT_RSES(router_cli_ses);
/**
* Lock router client session for secure read of router session members.
* Note that this could be done without lock by using version #
*/
if (!rses_begin_locked_router_action(router_cli_ses))
if (router_cli_ses->rses_closed)
{
gwbuf_free(writebuf);
goto lock_failed;
return;
}
/** Holding lock ensures that router session remains open */
ss_dassert(backend_dcb->session != NULL);
client_dcb = backend_dcb->session->client_dcb;
/** Unlock */
rses_end_locked_router_action(router_cli_ses);
/**
* 1. Check if backend received reply to sescmd.
* 2. Check sescmd's state whether OK_PACKET has been
@ -704,32 +691,10 @@ static void clientReply(MXS_ROUTER *instance,
* 3. If reply for this sescmd is sent, lock property cursor
* and
*/
if (client_dcb == NULL)
{
gwbuf_free(writebuf);
/** Log that client was closed before reply */
goto lock_failed;
}
/** Lock router session */
if (!rses_begin_locked_router_action(router_cli_ses))
{
/** Log to debug that router was closed */
goto lock_failed;
}
bref = get_bref_from_dcb(router_cli_ses, backend_dcb);
#if !defined(FOR_BUG548_FIX_ONLY)
/** This makes the issue becoming visible in poll.c */
if (bref == NULL)
{
/** Unlock router session */
rses_end_locked_router_action(router_cli_ses);
goto lock_failed;
}
#endif
backend_ref_t *bref = get_bref_from_dcb(router_cli_ses, backend_dcb);
CHK_BACKEND_REF(bref);
scur = &bref->bref_sescmd_cur;
sescmd_cursor_t *scur = &bref->bref_sescmd_cur;
/** Statement was successfully executed, free the stored statement */
session_clear_stmt(backend_dcb->session);
@ -790,15 +755,7 @@ static void clientReply(MXS_ROUTER *instance,
/** Write reply to client DCB */
MXS_SESSION_ROUTE_REPLY(backend_dcb->session, writebuf);
}
/** Unlock router session */
rses_end_locked_router_action(router_cli_ses);
/** Lock router session */
if (!rses_begin_locked_router_action(router_cli_ses))
{
/** Log to debug that router was closed */
goto lock_failed;
}
/** There is one pending session command to be executed. */
if (sescmd_cursor_is_active(scur))
{
@ -849,11 +806,6 @@ static void clientReply(MXS_ROUTER *instance,
gwbuf_free(bref->bref_pending_cmd);
bref->bref_pending_cmd = NULL;
}
/** Unlock router session */
rses_end_locked_router_action(router_cli_ses);
lock_failed:
return;
}
@ -878,64 +830,6 @@ static uint64_t getCapabilities(MXS_ROUTER* instance)
* functions are not intended for use outside the read write split router.
*/
/**
* @brief Acquires lock to router client session if it is not closed.
*
* Parameters:
* @param rses - in, use
*
*
* @return true if router session was not closed. If return value is true
* it means that router is locked, and must be unlocked later. False, if
* router was closed before lock was acquired.
*
*/
bool rses_begin_locked_router_action(ROUTER_CLIENT_SES *rses)
{
bool succp = false;
if (rses == NULL)
{
return false;
}
CHK_CLIENT_RSES(rses);
if (rses->rses_closed)
{
goto return_succp;
}
spinlock_acquire(&rses->rses_lock);
if (rses->rses_closed)
{
spinlock_release(&rses->rses_lock);
goto return_succp;
}
succp = true;
return_succp:
return succp;
}
/** to be inline'd */
/**
* @brief Releases router client session lock.
*
* Parameters:
* @param rses - <usage>
* <description>
*
* @return void
*
*/
void rses_end_locked_router_action(ROUTER_CLIENT_SES *rses)
{
CHK_CLIENT_RSES(rses);
spinlock_release(&rses->rses_lock);
}
/*
* @brief Clear one or more bits in the backend reference state
*
@ -1158,6 +1052,10 @@ backend_ref_t *get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb)
{
bref = NULL;
}
/** We should always have a valid backend reference */
ss_dassert(bref);
return bref;
}
@ -1366,7 +1264,7 @@ static void handleError(MXS_ROUTER *instance,
CHK_DCB(problem_dcb);
if (!rses_begin_locked_router_action(rses))
if (rses->rses_closed)
{
/** Session is already closed */
problem_dcb->dcb_errhandle_called = true;
@ -1383,7 +1281,6 @@ static void handleError(MXS_ROUTER *instance,
* be safe with the code as it stands on 9 Sept 2015 - MNB
*/
*succp = true;
rses_end_locked_router_action(rses);
return;
}
else
@ -1521,8 +1418,6 @@ static void handleError(MXS_ROUTER *instance,
break;
}
}
rses_end_locked_router_action(rses);
}
/**
@ -1642,7 +1537,6 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst,
bool succp;
myrses = *rses;
ss_dassert(SPINLOCK_IS_LOCKED(&myrses->rses_lock));
ses = backend_dcb->session;
CHK_SESSION(ses);

2
server/modules/routing/readwritesplit/readwritesplit.h
View File

@ -307,7 +307,6 @@ struct router_client_session
#if defined(SS_DEBUG)
skygw_chk_t rses_chk_top;
#endif
SPINLOCK rses_lock; /*< protects rses_deleted */
bool rses_closed; /*< true when closeSession is called */
rses_property_t* rses_properties[RSES_PROP_TYPE_COUNT]; /*< Properties listed by their type */
backend_ref_t* rses_master_ref;
@ -349,7 +348,6 @@ typedef struct
typedef struct router_instance
{
SERVICE* service; /*< Pointer to service */
SPINLOCK lock; /*< Lock for the instance data */
rwsplit_config_t rwsplit_config; /*< expanded config info from SERVICE */
int rwsplit_version; /*< version number for router's config */
ROUTER_STATS stats; /*< Statistics for this router */

3
server/modules/routing/readwritesplit/rwsplit_internal.h
View File

@ -56,7 +56,6 @@ bool handle_target_is_all(route_target_t route_target,
GWBUF *querybuf, int packet_type, qc_query_type_t qtype);
int determine_packet_type(GWBUF *querybuf, bool *non_empty_packet);
void log_transaction_status(ROUTER_CLIENT_SES *rses, GWBUF *querybuf, qc_query_type_t qtype);
void session_lock_failure_handling(GWBUF *querybuf, int packet_type, qc_query_type_t qtype);
bool is_packet_a_one_way_message(int packet_type);
sescmd_cursor_t *backend_ref_get_sescmd_cursor(backend_ref_t *bref);
bool is_packet_a_query(int packet_type);
@ -65,8 +64,6 @@ bool send_readonly_error(DCB *dcb);
/*
* The following are implemented in readwritesplit.c
*/
bool rses_begin_locked_router_action(ROUTER_CLIENT_SES *rses);
void rses_end_locked_router_action(ROUTER_CLIENT_SES *rses);
void bref_clear_state(backend_ref_t *bref, bref_state_t state);
void bref_set_state(backend_ref_t *bref, bref_state_t state);
int router_handle_state_switch(DCB *dcb, DCB_REASON reason, void *data);

26
server/modules/routing/readwritesplit/rwsplit_mysql.c
View File

@ -287,32 +287,6 @@ handle_target_is_all(route_target_t route_target,
return result;
}
/* This is MySQL specific */
/**
* @brief Write an error message to the log indicating failure
*
* Used when an attempt to lock the router session fails.
*
* @param querybuf Query buffer containing packet
* @param packet_type Integer (enum) indicating type of packet
* @param qtype Query type
*/
void
session_lock_failure_handling(GWBUF *querybuf, int packet_type, qc_query_type_t qtype)
{
if (packet_type != MYSQL_COM_QUIT)
{
/* NOTE: modutil_get_query is MySQL specific */
char *query_str = modutil_get_query(querybuf);
MXS_ERROR("Can't route %s:%s:\"%s\" to "
"backend server. Router is closed.",
STRPACKETTYPE(packet_type), STRQTYPE(qtype),
(query_str == NULL ? "(empty)" : query_str));
MXS_FREE(query_str);
}
}
/*
* Probably MySQL specific because of modutil function
*/

34
server/modules/routing/readwritesplit/rwsplit_route_stmt.c
View File

@ -70,16 +70,10 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
/* packet_type is a problem as it is MySQL specific */
packet_type = determine_packet_type(querybuf, &non_empty_packet);
qtype = determine_query_type(querybuf, packet_type, non_empty_packet);
if (non_empty_packet)
{
/** This might not be absolutely necessary as some parts of the code
* can only be executed by one thread at a time. */
if (!rses_begin_locked_router_action(rses))
{
return false;
}
handle_multi_temp_and_load(rses, querybuf, packet_type, (int *)&qtype);
rses_end_locked_router_action(rses);
if (MXS_LOG_PRIORITY_IS_ENABLED(LOG_INFO))
{
@ -116,7 +110,7 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
{
succp = handle_target_is_all(route_target, inst, rses, querybuf, packet_type, qtype);
}
else if (rses_begin_locked_router_action(rses))
else
{
/* Now we have a lock on the router session */
bool store_stmt = false;
@ -152,12 +146,6 @@ bool route_single_stmt(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses,
ss_dassert(!store_stmt || TARGET_IS_SLAVE(route_target));
handle_got_target(inst, rses, querybuf, target_dcb, store_stmt);
}
rses_end_locked_router_action(rses);
}
else
{
session_lock_failure_handling(querybuf, packet_type, qtype);
succp = false;
}
return succp;
@ -214,12 +202,6 @@ bool route_session_write(ROUTER_CLIENT_SES *router_cli_ses,
{
int rc;
/** Lock router session */
if (!rses_begin_locked_router_action(router_cli_ses))
{
goto return_succp;
}
for (i = 0; i < router_cli_ses->rses_nbackends; i++)
{
DCB *dcb = backend_ref[i].bref_dcb;
@ -244,15 +226,9 @@ bool route_session_write(ROUTER_CLIENT_SES *router_cli_ses,
}
}
}
rses_end_locked_router_action(router_cli_ses);
gwbuf_free(querybuf);
goto return_succp;
}
/** Lock router session */
if (!rses_begin_locked_router_action(router_cli_ses))
{
goto return_succp;
}
if (router_cli_ses->rses_nbackends <= 0)
{
@ -318,7 +294,6 @@ bool route_session_write(ROUTER_CLIENT_SES *router_cli_ses,
if ((prop = rses_property_init(RSES_PROP_TYPE_SESCMD)) == NULL)
{
MXS_ERROR("Router session property initialization failed");
rses_end_locked_router_action(router_cli_ses);
return false;
}
@ -328,7 +303,6 @@ bool route_session_write(ROUTER_CLIENT_SES *router_cli_ses,
if (rses_property_add(router_cli_ses, prop) != 0)
{
MXS_ERROR("Session property addition failed.");
rses_end_locked_router_action(router_cli_ses);
return false;
}
@ -387,9 +361,6 @@ bool route_session_write(ROUTER_CLIENT_SES *router_cli_ses,
atomic_add(&router_cli_ses->rses_nsescmd, 1);
/** Unlock router session */
rses_end_locked_router_action(router_cli_ses);
return_succp:
/**
* Routing must succeed to all backends that are used.
@ -1364,7 +1335,6 @@ int rses_property_add(ROUTER_CLIENT_SES *rses, rses_property_t *prop)
CHK_CLIENT_RSES(rses);
CHK_RSES_PROP(prop);
ss_dassert(SPINLOCK_IS_LOCKED(&rses->rses_lock));
prop->rses_prop_rsession = rses;
p = rses->rses_properties[prop->rses_prop_type];

18
server/modules/routing/readwritesplit/rwsplit_session_cmd.c
View File

@ -60,8 +60,6 @@ mysql_sescmd_t *rses_property_get_sescmd(rses_property_t *prop)
}
CHK_RSES_PROP(prop);
ss_dassert(prop->rses_prop_rsession == NULL ||
SPINLOCK_IS_LOCKED(&prop->rses_prop_rsession->rses_lock));
sescmd = &prop->rses_prop_data.sescmd;
@ -134,14 +132,9 @@ GWBUF *sescmd_cursor_process_replies(GWBUF *replybuf,
backend_ref_t *bref,
bool *reconnect)
{
mysql_sescmd_t *scmd;
sescmd_cursor_t *scur;
ROUTER_CLIENT_SES *ses;
scur = &bref->bref_sescmd_cur;
ss_dassert(SPINLOCK_IS_LOCKED(&(scur->scmd_cur_rses->rses_lock)));
scmd = sescmd_cursor_get_command(scur);
ses = (*scur->scmd_cur_ptr_property)->rses_prop_rsession;
sescmd_cursor_t *scur = &bref->bref_sescmd_cur;
mysql_sescmd_t *scmd = sescmd_cursor_get_command(scur);
ROUTER_CLIENT_SES *ses = (*scur->scmd_cur_ptr_property)->rses_prop_rsession;
CHK_GWBUF(replybuf);
/**
@ -273,7 +266,6 @@ mysql_sescmd_t *sescmd_cursor_get_command(sescmd_cursor_t *scur)
{
mysql_sescmd_t *scmd;
ss_dassert(SPINLOCK_IS_LOCKED(&(scur->scmd_cur_rses->rses_lock)));
scur->scmd_cur_cmd = rses_property_get_sescmd(*scur->scmd_cur_ptr_property);
CHK_MYSQL_SESCMD(scur->scmd_cur_cmd);
@ -293,7 +285,6 @@ bool sescmd_cursor_is_active(sescmd_cursor_t *sescmd_cursor)
MXS_ERROR("[%s] Error: NULL parameter.", __FUNCTION__);
return false;
}
ss_dassert(SPINLOCK_IS_LOCKED(&sescmd_cursor->scmd_cur_rses->rses_lock));
succp = sescmd_cursor->scmd_cur_active;
return succp;
@ -303,7 +294,6 @@ bool sescmd_cursor_is_active(sescmd_cursor_t *sescmd_cursor)
void sescmd_cursor_set_active(sescmd_cursor_t *sescmd_cursor,
bool value)
{
ss_dassert(SPINLOCK_IS_LOCKED(&sescmd_cursor->scmd_cur_rses->rses_lock));
/** avoid calling unnecessarily */
ss_dassert(sescmd_cursor->scmd_cur_active != value);
sescmd_cursor->scmd_cur_active = value;
@ -420,8 +410,6 @@ static bool sescmd_cursor_next(sescmd_cursor_t *scur)
ss_dassert(scur != NULL);
ss_dassert(*(scur->scmd_cur_ptr_property) != NULL);
ss_dassert(SPINLOCK_IS_LOCKED(
&(*(scur->scmd_cur_ptr_property))->rses_prop_rsession->rses_lock));
/** Illegal situation */
if (scur == NULL || *scur->scmd_cur_ptr_property == NULL ||