From cd92b6d4d7c87dcb99dfe301b2635396d3497f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 29 Oct 2018 16:05:04 +0200 Subject: [PATCH] MXS-2122: Close listener socket on destruction Even though directly closing the socket is not very neat in the architectural sense of things, it allows the best of both worlds: the socket is instantly closed and is open for reuse while the listener struct is still available as a reference. This change needs to be revised when the listeners are refactored into separate objects. Updated documentation to reflect the change in behavior. --- Documentation/REST-API/Resources-Service.md | 6 +++--- Documentation/Reference/MaxAdmin.md | 12 +++++------- Documentation/Reference/MaxCtrl.md | 5 +---- maxctrl/lib/destroy.js | 6 +----- server/core/dcb.cc | 5 +++++ server/core/service.cc | 5 +++++ 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Documentation/REST-API/Resources-Service.md b/Documentation/REST-API/Resources-Service.md index 9615eb3d5..bb9581dfd 100644 --- a/Documentation/REST-API/Resources-Service.md +++ b/Documentation/REST-API/Resources-Service.md @@ -323,9 +323,6 @@ all the listeners pointing to the service have been destroyed. This means that the `data.relationships` must be an empty object and `data.attributes.listeners` must be an empty array in order for the service to qualify for destruction. -Once a service is destroyed, any listeners associated with it will be freed -after which the ports can be reused by other listeners. - If there are open client connections that use the service when it is destroyed, they are allowed to gracefully close before the service is destroyed. This means that the destruction of a service can be acknowledged via the REST API before @@ -466,6 +463,9 @@ DELETE /v1/services/:service/listeners/:name In the URI , the _:name_ must map to a listener and the _:service_ must map to a service. Both names must have all whitespace replaced with hyphens. +When a listener is destroyed, the network port it listens on is available for +reuse. + #### Response Listener is destroyed: diff --git a/Documentation/Reference/MaxAdmin.md b/Documentation/Reference/MaxAdmin.md index 247edad8d..bc1dec8ae 100644 --- a/Documentation/Reference/MaxAdmin.md +++ b/Documentation/Reference/MaxAdmin.md @@ -1547,8 +1547,8 @@ Example: create listener my-service my-new-listener 192.168.0.101 4006 You can destroy created listeners with the `destroy listener` command. This will remove the persisted configuration and it will not be created on the next -startup. The listener is stopped but it will remain a part of the runtime -configuration until the next restart. +startup. The listener is destroyed and the listening port is immediately +available for reuse. ``` destroy listener - Destroy a listener @@ -1558,7 +1558,7 @@ Usage: destroy listener SERVICE NAME Parameters: NAME Listener to destroy -The listener is stopped and it will be removed on the next restart of MaxScale +The listener is deleted Example: destroy listener my-listener ``` @@ -1615,9 +1615,7 @@ Example: alter monitor my-monitor user=maxuser password=maxpwd ### Destroying Monitors To destroy a monitor, use the `destroy monitor` command. All servers need to be -removed from the monitor before it can be destroyed. Only created monitors -should be destroyed and they will remain a part of the runtime configuration -until the next restart. +removed from the monitor before it can be destroyed. ``` destroy monitor - Destroy a monitor @@ -1627,7 +1625,7 @@ Usage: destroy monitor NAME Parameters: NAME Monitor to destroy -The monitor is stopped and it will be removed on the next restart of MaxScale +The monitor is destroyed Example: destroy monitor my-monitor ``` diff --git a/Documentation/Reference/MaxCtrl.md b/Documentation/Reference/MaxCtrl.md index 04464cc0d..04099b05b 100644 --- a/Documentation/Reference/MaxCtrl.md +++ b/Documentation/Reference/MaxCtrl.md @@ -523,10 +523,7 @@ The monitor must be unlinked from all servers before it can be destroyed. `Usage: destroy listener ` -Destroying a monitor causes it to be removed on the next restart. Destroying a -listener at runtime stops it from accepting new connections but it will still be -bound to the listening socket. This means that new listeners cannot be created -to replace destroyed listeners without restarting MaxScale. +Destroying a listener closes the listening socket, opening it up for reuse. ### destroy service diff --git a/maxctrl/lib/destroy.js b/maxctrl/lib/destroy.js index d91901055..6d1792554 100644 --- a/maxctrl/lib/destroy.js +++ b/maxctrl/lib/destroy.js @@ -34,11 +34,7 @@ exports.builder = function(yargs) { }) }) .command('listener ', 'Destroy an unused listener', function(yargs) { - return yargs.epilog('Destroying a monitor causes it to be removed on the next restart. ' + - 'Destroying a listener at runtime stops it from accepting new ' + - 'connections but it will still be bound to the listening socket. This ' + - 'means that new listeners cannot be created to replace destroyed listeners ' + - 'without restarting MaxScale.') + return yargs.epilog('Destroying a listener closes the listening socket, opening it up for reuse.') .usage('Usage: destroy listener ') }, function(argv) { maxctrl(argv, function(host) { diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 468f0bbfc..36bfa224e 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -1274,6 +1274,11 @@ void dcb_final_close(DCB* dcb) } } } + else + { + // Only listeners are closed with a fd of -1 + mxb_assert(dcb->dcb_role == DCB_ROLE_SERVICE_LISTENER); + } dcb->state = DCB_STATE_DISCONNECTED; dcb_remove_from_list(dcb); diff --git a/server/core/service.cc b/server/core/service.cc index 3d1e4c258..951c7fe23 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -751,6 +751,11 @@ bool service_remove_listener(Service* service, const char* target) { listener->listener->session->state = SESSION_STATE_LISTENER_STOPPED; rval = true; + + // TODO: This is not pretty but it works, revise when listeners are refactored. This is + // thread-safe as the listener is freed on the same thread that closes the socket. + close(listener->listener->fd); + listener->listener->fd = -1; } break; }