MXS-2710: Move client_count handling into client DCB

Due to the fact that both client connections and listeners use sessions in
2.3, the client_count tracking must be done inside the client DCB. In
addition to this, the max_connections check didn't take the current
pending connection into account which caused an off-by-one error.

This commit fixes the connection_limit test failure that was introduced by
commit 6306519e5e75575ba083ee2f0edfe7e624da5d26.
This commit is contained in:
Markus Mäkelä
2020-01-10 13:29:36 +02:00
parent 4918914042
commit 1d8139587b
3 changed files with 23 additions and 30 deletions

View File

@ -56,6 +56,7 @@
#include "internal/modules.h"
#include "internal/session.h"
#include "internal/service.hh"
using maxscale::RoutingWorker;
using maxbase::Worker;
@ -218,6 +219,11 @@ DCB* dcb_alloc(dcb_role_t role, SERV_LISTENER* listener)
newdcb->poll.owner = RoutingWorker::get_current();
}
if (role == DCB_ROLE_CLIENT_HANDLER)
{
mxb::atomic::add(&listener->service->client_count, 1, mxb::atomic::RELAXED);
}
return newdcb;
}
@ -282,6 +288,21 @@ void dcb_free_all_memory(DCB* dcb)
this_thread.current_dcb = NULL;
}
if (dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER)
{
Service* service = static_cast<Service*>(dcb->service);
bool should_destroy = !mxb::atomic::load(&service->active);
if (mxb::atomic::add(&service->client_count, -1) == 1 && should_destroy)
{
// Destroy the service in the main routing worker thread
mxs::RoutingWorker* main_worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN);
main_worker->execute([service]() {
service_free(service);
}, Worker::EXECUTE_AUTO);
}
}
DCB_CALLBACK* cb_dcb;
if (dcb->protocol)
@ -2518,7 +2539,7 @@ DCB* dcb_accept(DCB* dcb)
}
if (client_dcb->service->max_connections
&& client_dcb->service->client_count >= client_dcb->service->max_connections)
&& client_dcb->service->client_count > client_dcb->service->max_connections)
{
// TODO: If connections can be queued, this is the place to put the
// TODO: connection on that queue.

View File

@ -336,23 +336,6 @@ void session_close(MXS_SESSION* session)
}
}
class ServiceDestroyTask : public Worker::DisposableTask
{
public:
ServiceDestroyTask(Service* service)
: m_service(service)
{
}
void execute(Worker& worker) override
{
service_free(m_service);
}
private:
Service* m_service;
};
/**
* Deallocate the specified session
*
@ -1253,7 +1236,6 @@ Session::Session(SERVICE* service)
mxb::atomic::add(&service->stats.n_current, 1, mxb::atomic::RELAXED);
mxb_assert(service->stats.n_current >= 0);
mxb::atomic::add(&service->client_count, 1, mxb::atomic::RELAXED);
}
Session::~Session()
@ -1271,17 +1253,6 @@ Session::~Session()
mxb::atomic::add(&service->stats.n_current, -1, mxb::atomic::RELAXED);
mxb_assert(service->stats.n_current >= 0);
bool should_destroy = !mxb::atomic::load(&service->active);
if (mxb::atomic::add(&service->client_count, -1) == 1 && should_destroy)
{
// Destroy the service in the main routing worker thread
mxs::RoutingWorker* main_worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN);
main_worker->execute(
std::unique_ptr<ServiceDestroyTask>(new ServiceDestroyTask(static_cast<Service*>(service))),
Worker::EXECUTE_AUTO);
}
}
namespace

View File

@ -53,6 +53,7 @@ static int test1()
SERVICE service;
service.routerModule = (char*)"required by a check in dcb.cc";
dummy.service = &service;
/* Poll tests */
fprintf(stderr,