From e14b29baf90f7ab3f62ea5a0314b0b1da0e5a7d5 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sun, 21 Jun 2015 12:51:54 +0300 Subject: [PATCH 1/7] Fix to MXS-212: https://mariadb.atlassian.net/browse/MXS-212 The listener DCB is now properly closed instead of just being removed from the poll set. --- server/core/dcb.c | 8 +++++++- server/core/poll.c | 3 ++- server/core/service.c | 19 +++++++++---------- server/modules/protocol/mysql_client.c | 2 +- server/modules/protocol/mysql_common.c | 5 ++++- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7f3651953..fc9329524 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1949,13 +1949,15 @@ dcb_close(DCB *dcb) } ss_dassert(dcb->state == DCB_STATE_POLLING || + dcb->state == DCB_STATE_LISTENING || dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE); /*< * Stop dcb's listening and modify state accordingly. */ - if (dcb->state == DCB_STATE_POLLING) + if (dcb->state == DCB_STATE_POLLING || + dcb->state == DCB_STATE_LISTENING) { rc = poll_remove_dcb(dcb); @@ -2428,6 +2430,10 @@ static bool dcb_set_state_nomutex( case DCB_STATE_POLLING: /*< ok to try but state can't change */ succp = true; break; + case DCB_STATE_LISTENING: + dcb->state = new_state; + succp = true; + break; default: ss_dassert(old_state != NULL); break; diff --git a/server/core/poll.c b/server/core/poll.c index 377310cb0..9a1a5565d 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -330,7 +330,8 @@ poll_remove_dcb(DCB *dcb) CHK_DCB(dcb); /*< It is possible that dcb has already been removed from the set */ - if (dcb->state != DCB_STATE_POLLING) + if (dcb->state != DCB_STATE_POLLING && + dcb->state != DCB_STATE_LISTENING) { if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) diff --git a/server/core/service.c b/server/core/service.c index b0959e6c3..c11c0bddc 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,11 +534,13 @@ int listeners = 0; port = service->ports; while (port) { - poll_remove_dcb(port->listener); - port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; + if(port->listener) + { + dcb_close(port->listener); + port->listener = NULL; listeners++; - - port = port->next; + } + port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -562,13 +564,10 @@ int listeners = 0; port = service->ports; while (port) { - if (poll_add_dcb(port->listener) == 0) { - port->listener->session->state = SESSION_STATE_LISTENER; - listeners++; - } - port = port->next; + listeners += serviceStartPort(service,port); + port = port->next; } - + service->state = SERVICE_STATE_STARTED; return listeners; } diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index c3e463139..21a11dc42 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1771,7 +1771,7 @@ gw_client_close(DCB *dcb) dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) { - if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(protocol); + if (!DCB_IS_CLONE(dcb) && protocol) CHK_PROTOCOL(protocol); } #endif LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 83e2912e5..eb044c1fb 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -128,7 +128,10 @@ void mysql_protocol_done ( MySQLProtocol* p; server_command_t* scmd; server_command_t* scmd2; - + + if(dcb->protocol == NULL) + return; + p = (MySQLProtocol *)dcb->protocol; spinlock_acquire(&p->protocol_lock); From c22c6ea46aaaad1cfba993ccd19a41cbea4abe6e Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 09:15:06 +0300 Subject: [PATCH 2/7] ServiceStop only removed DCBs from the polling system This removes the need to establish new DCBs for each of the listeners while still blocking new session creation for a service which is shut down. The client will not receive an error and the connection will be accepted when the service is restarted. --- server/core/service.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index c11c0bddc..cfce442b8 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,12 +534,8 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener) - { - dcb_close(port->listener); - port->listener = NULL; + if(poll_remove_dcb(port->listener) == 0) listeners++; - } port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -564,7 +560,8 @@ int listeners = 0; port = service->ports; while (port) { - listeners += serviceStartPort(service,port); + if(poll_add_dcb(port->listener) == 0) + listeners++; port = port->next; } service->state = SERVICE_STATE_STARTED; From dc43a7d9da855015b1013980d1227c76c845e015 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:25:59 +0300 Subject: [PATCH 3/7] Removed unnecessary code from dcb_close and dcb_set_state_nomutex. --- server/core/dcb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index fc9329524..7f3651953 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1949,15 +1949,13 @@ dcb_close(DCB *dcb) } ss_dassert(dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_LISTENING || dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE); /*< * Stop dcb's listening and modify state accordingly. */ - if (dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_LISTENING) + if (dcb->state == DCB_STATE_POLLING) { rc = poll_remove_dcb(dcb); @@ -2430,10 +2428,6 @@ static bool dcb_set_state_nomutex( case DCB_STATE_POLLING: /*< ok to try but state can't change */ succp = true; break; - case DCB_STATE_LISTENING: - dcb->state = new_state; - succp = true; - break; default: ss_dassert(old_state != NULL); break; From 3de7798fac7ffc74d36d50316e1449b3ab333bb2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:49:27 +0300 Subject: [PATCH 4/7] Added missing session state changes. --- server/core/service.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/core/service.c b/server/core/service.c index cfce442b8..bce7328cc 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -535,7 +535,10 @@ int listeners = 0; while (port) { if(poll_remove_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; listeners++; + } port = port->next; } service->state = SERVICE_STATE_STOPPED; @@ -561,7 +564,10 @@ int listeners = 0; while (port) { if(poll_add_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER; listeners++; + } port = port->next; } service->state = SERVICE_STATE_STARTED; From 3d7b050f6ff3df7cc939903a8db202e938168be3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 11:53:29 +0300 Subject: [PATCH 5/7] Removed unnecessary NULL checks in mysql_client and mysql_common --- server/modules/protocol/mysql_client.c | 2 +- server/modules/protocol/mysql_common.c | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 21a11dc42..c3e463139 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1771,7 +1771,7 @@ gw_client_close(DCB *dcb) dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ZOMBIE) { - if (!DCB_IS_CLONE(dcb) && protocol) CHK_PROTOCOL(protocol); + if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(protocol); } #endif LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index eb044c1fb..83e2912e5 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -128,10 +128,7 @@ void mysql_protocol_done ( MySQLProtocol* p; server_command_t* scmd; server_command_t* scmd2; - - if(dcb->protocol == NULL) - return; - + p = (MySQLProtocol *)dcb->protocol; spinlock_acquire(&p->protocol_lock); From 039cbff1811315606bdec20dee421d9521830fe1 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 16:45:10 +0300 Subject: [PATCH 6/7] Added missing null checks. --- server/core/service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/core/service.c b/server/core/service.c index bce7328cc..7966d33ec 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,7 +534,7 @@ int listeners = 0; port = service->ports; while (port) { - if(poll_remove_dcb(port->listener) == 0) + if(port->listener && poll_remove_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; listeners++; @@ -563,7 +563,7 @@ int listeners = 0; port = service->ports; while (port) { - if(poll_add_dcb(port->listener) == 0) + if(port->listener && poll_add_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER; listeners++; From c42d3d9f7ac086b5e012d9c2ee64f3b024c9ed17 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 23 Jun 2015 17:05:31 +0300 Subject: [PATCH 7/7] Added missing NULL checks. --- server/core/dcb.c | 3 +++ server/core/service.c | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7f3651953..07bb1dc93 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -2423,6 +2423,9 @@ static bool dcb_set_state_nomutex( case DCB_STATE_NOPOLLING: switch (new_state) { + /** Stopped services which are restarting will go from + * DCB_STATE_NOPOLLING to DCB_STATE_LISTENING.*/ + case DCB_STATE_LISTENING: case DCB_STATE_ZOMBIE: /*< fall through */ dcb->state = new_state; case DCB_STATE_POLLING: /*< ok to try but state can't change */ diff --git a/server/core/service.c b/server/core/service.c index 7966d33ec..24d51542d 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -534,10 +534,14 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener && poll_remove_dcb(port->listener) == 0) + if(port->listener && + port->listener->session->state == SESSION_STATE_LISTENER) { - port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; - listeners++; + if(poll_remove_dcb(port->listener) == 0) + { + port->listener->session->state = SESSION_STATE_LISTENER_STOPPED; + listeners++; + } } port = port->next; } @@ -563,11 +567,15 @@ int listeners = 0; port = service->ports; while (port) { - if(port->listener && poll_add_dcb(port->listener) == 0) + if(port->listener && + port->listener->session->state == SESSION_STATE_LISTENER_STOPPED) + { + if(poll_add_dcb(port->listener) == 0) { port->listener->session->state = SESSION_STATE_LISTENER; listeners++; } + } port = port->next; } service->state = SERVICE_STATE_STARTED;