MXS-2196: Remove the dummy session

As each connection now immediately gets a session the dummy session is no
longer required. The next step would be to combine parts of the session
and the client DCB into one entity. This would prevent the possibility of
a client DCB with no associated session. Backend DCBs are different as
they can move from one session to another when the persistent connection
pool is in use.

include/maxscale/session.hh

@@ -41,7 +41,6 @@ typedef enum
     SESSION_STATE_LISTENER_STOPPED, /*< for listener session */
     SESSION_STATE_TO_BE_FREED,      /*< ready to be freed as soon as there are no references */
     SESSION_STATE_FREE,             /*< for all sessions */
-    SESSION_STATE_DUMMY             /*< dummy session for consistency */
 } mxs_session_state_t;
 
 #define STRSESSIONSTATE(s) \
@@ -79,14 +78,7 @@ typedef enum
                                                                                 SESSION_STATE_FREE \
                                                                                 ? \
                                                                                 "SESSION_STATE_TO_BE_FREE"   \
-                                                                                : (( \
+                                                                                :  "SESSION_STATE_UNKNOWN"))))))))
-                                                                                       s) \
-                                                                                   == \
-                                                                                   SESSION_STATE_DUMMY \
-                                                                                   ? \
-                                                                                   "SESSION_STATE_DUMMY"   \
-                                                                                   : \
-                                                                                   "SESSION_STATE_UNKNOWN")))))))))
 
 typedef enum
 {
@@ -207,6 +199,9 @@ typedef char* (* session_variable_handler_t)(void* context,
  */
 struct MXS_SESSION
 {
+    MXS_SESSION(DCB* client_dcb);
+    ~MXS_SESSION();
+
     mxs_session_state_t state;      /*< Current descriptor state */
     uint64_t            ses_id;     /*< Unique session identifier */
     DCB*                client_dcb; /*< The client connection */
@@ -304,13 +299,6 @@ MXS_SESSION* session_alloc(SERVICE*, DCB*);
  */
 bool session_start(MXS_SESSION* session);
 
-MXS_SESSION* session_set_dummy(DCB*);
-
-static inline bool session_is_dummy(MXS_SESSION* session)
-{
-    return session->state == SESSION_STATE_DUMMY;
-}
-
 const char* session_get_remote(const MXS_SESSION*);
 const char* session_get_user(const MXS_SESSION*);
 

server/core/dcb.cc

@@ -150,14 +150,13 @@ static MXB_WORKER* get_dcb_owner(dcb_role_t role)
 DCB::DCB(dcb_role_t role, const SListener& listener, SERVICE* service)
     : MXB_POLL_DATA{dcb_poll_handler, get_dcb_owner(role)}
     , dcb_role(role)
+    , session(nullptr)
     , listener(listener)
     , high_water(config_writeq_high_water())
     , low_water(config_writeq_low_water())
     , service(service)
     , last_read(mxs_clock())
 {
-    session_set_dummy(this);
-
     // TODO: Remove DCB_ROLE_INTERNAL to always have a valid listener
     if (listener)
     {
@@ -245,25 +244,22 @@ static void dcb_final_free(DCB* dcb)
          */
         MXS_SESSION* local_session = dcb->session;
         dcb->session = NULL;
-        if (SESSION_STATE_DUMMY != local_session->state)
+        if (dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER)
         {
-            if (dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER)
+            session_unlink_backend_dcb(local_session, dcb);
-            {
+        }
-                session_unlink_backend_dcb(local_session, dcb);
+        else
-            }
+        {
-            else
+            /**
-            {
+             * The client DCB is only freed once all other DCBs that the session
-                /**
+             * uses have been freed. This will guarantee that the authentication
-                 * The client DCB is only freed once all other DCBs that the session
+             * data will be usable for all DCBs even if the client DCB has already
-                 * uses have been freed. This will guarantee that the authentication
+             * been closed.
-                 * data will be usable for all DCBs even if the client DCB has already
+             */
-                 * been closed.
-                 */
 
-                mxb_assert(dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER || dcb->dcb_role == DCB_ROLE_INTERNAL);
+            mxb_assert(dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER || dcb->dcb_role == DCB_ROLE_INTERNAL);
-                session_put_ref(local_session);
+            session_put_ref(local_session);
-                return;
+            return;
-            }
         }
     }
 
@@ -1237,18 +1233,8 @@ static bool dcb_maybe_add_persistent(DCB* dcb)
         MXS_DEBUG("Adding DCB to persistent pool, user %s.", dcb->user);
         dcb->was_persistent = false;
         dcb->persistentstart = time(NULL);
-        if (dcb->session)
+        session_unlink_backend_dcb(dcb->session, dcb);
-        /*<
+        dcb->session = nullptr;
-         * Terminate client session.
-         */
-        {
-            MXS_SESSION* local_session = dcb->session;
-            session_set_dummy(dcb);
-            if (SESSION_STATE_DUMMY != local_session->state)
-            {
-                session_unlink_backend_dcb(local_session, dcb);
-            }
-        }
 
         while ((loopcallback = dcb->callbacks) != NULL)
         {
@@ -1563,7 +1549,7 @@ void dprintDCB(DCB* pdcb, DCB* dcb)
         dcb_printf(pdcb, "\tProtocol:                   %s\n", dcb->listener->protocol());
     }
 
-    if (dcb->session && dcb->session->state != SESSION_STATE_DUMMY)
+    if (dcb->session)
     {
         dcb_printf(pdcb, "\tOwning Session:     %" PRIu64 "\n", dcb->session->ses_id);
     }
@@ -2493,9 +2479,7 @@ public:
              dcb && atomic_load_int32(&m_more);
              dcb = dcb->thread.next)
         {
-            mxb_assert(dcb->session);
+            if (dcb->session)
-
-            if (dcb->session->state != SESSION_STATE_DUMMY)
             {
                 if (!m_func(dcb, m_data))
                 {
@@ -2503,6 +2487,10 @@ public:
                     break;
                 }
             }
+            else
+            {
+                mxb_assert_message(dcb->persistentstart > 0, "The DCB must be in a connection pool");
+            }
         }
     }
 

server/core/internal/session.hh

@@ -164,7 +164,7 @@ public:
 
     using FilterList = std::vector<SessionFilter>;
 
-    Session(SERVICE* service);
+    Session(SERVICE* service, DCB* client_dcb);
     ~Session();
 
     bool setup_filters(Service* service);

server/core/session.cc

@@ -69,18 +69,8 @@ struct
     SESSION_DUMP_STATEMENTS_NEVER
 };
 
-static MXS_SESSION dummy_session()
-{
-    MXS_SESSION session = {};
-    session.state = SESSION_STATE_DUMMY;
-    session.refcount = 1;
-    return session;
 }
 
-}
-
-static MXS_SESSION session_dummy_struct = dummy_session();
-
 static void         session_initialize(void* session);
 static int          session_setup_filters(MXS_SESSION* session);
 static void         session_simple_free(MXS_SESSION* session, DCB* dcb);
@@ -98,50 +88,33 @@ static void session_deliver_response(MXS_SESSION* session);
  */
 static int session_reply(MXS_FILTER* inst, MXS_FILTER_SESSION* session, GWBUF* data);
 
+MXS_SESSION::MXS_SESSION(DCB* client_dcb)
+    : state(SESSION_STATE_READY)
+    , ses_id(session_get_next_id())
+    , client_dcb(client_dcb)
+    , router_session(nullptr)
+    , stats{time(0)}
+    , service(client_dcb->service)
+    , head{}
+    , tail{}
+    , refcount(1)
+    , trx_state(SESSION_TRX_INACTIVE)
+    , autocommit(config_get_global_options()->qc_sql_mode == QC_SQL_MODE_ORACLE ? false : true)
+    , client_protocol_data(0)
+    , qualifies_for_pooling(false)
+    , response{}
+    , close_reason(SESSION_CLOSE_NONE)
+    , load_active(false)
+{
+}
+
+MXS_SESSION::~MXS_SESSION()
+{
+}
+
 MXS_SESSION* session_alloc(SERVICE* service, DCB* client_dcb)
 {
-    Session* session = new (std::nothrow) Session(service);
+    return new(std::nothrow) Session(service, client_dcb);
-
-    if (session == nullptr)
-    {
-        return NULL;
-    }
-
-    session->state = SESSION_STATE_READY;
-    session->ses_id = session_get_next_id();
-    session->client_dcb = client_dcb;
-    session->router_session = NULL;
-    session->stats.connect = time(0);
-    session->service = service;
-    memset(&session->head, 0, sizeof(session->head));
-    memset(&session->tail, 0, sizeof(session->tail));
-    session->load_active = false;
-
-    /*<
-     * Associate the session to the client DCB and set the reference count on
-     * the session to indicate that there is a single reference to the
-     * session. There is no need to protect this or use atomic add as the
-     * session has not been made available to the other threads at this
-     * point.
-     *
-     * Note: Strictly speaking, we do have to increment it with a relaxed atomic
-     *       operation but in practice it doesn't matter.
-     */
-    session->refcount = 1;
-    session->trx_state = SESSION_TRX_INACTIVE;
-
-    MXS_CONFIG* config = config_get_global_options();
-    // If MaxScale is running in Oracle mode, then autocommit needs to
-    // initially be off.
-    bool autocommit = (config->qc_sql_mode == QC_SQL_MODE_ORACLE) ? false : true;
-    session_set_autocommit(session, autocommit);
-
-    session->client_protocol_data = 0;
-    session->qualifies_for_pooling = false;
-    memset(&session->response, 0, sizeof(session->response));
-    session->close_reason = SESSION_CLOSE_NONE;
-
-    return session;
 }
 
 bool session_start(MXS_SESSION* session)
@@ -200,21 +173,6 @@ bool session_start(MXS_SESSION* session)
     return true;
 }
 
-/**
- * Allocate a dummy session so that DCBs can always have sessions.
- *
- * Only one dummy session exists, it is statically declared
- *
- * @param client_dcb    The client side DCB
- * @return              The dummy created session
- */
-MXS_SESSION* session_set_dummy(DCB* client_dcb)
-{
-    MXS_SESSION* session = &session_dummy_struct;
-    client_dcb->session = session;
-    return session;
-}
-
 void session_link_backend_dcb(MXS_SESSION* session, DCB* dcb)
 {
     mxb_assert(dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
@@ -255,11 +213,7 @@ static void session_simple_free(MXS_SESSION* session, DCB* dcb)
     }
     if (session)
     {
-        if (SESSION_STATE_DUMMY == session->state)
+        if (session->router_session)
-        {
-            return;
-        }
-        if (session && session->router_session)
         {
             session->service->router->freeSession(session->service->router_instance,
                                                   session->router_session);
@@ -402,8 +356,7 @@ void printAllSessions()
 /** Callback for dprintAllSessions */
 bool dprintAllSessions_cb(DCB* dcb, void* data)
 {
-    if (dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER
+    if (dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER)
-        && dcb->session->state != SESSION_STATE_DUMMY)
     {
         DCB* out_dcb = (DCB*)data;
         dprintSession(out_dcb, dcb->session);
@@ -520,9 +473,6 @@ const char* session_state(mxs_session_state_t state)
     case SESSION_STATE_ALLOC:
         return "Session Allocated";
 
-    case SESSION_STATE_DUMMY:
-        return "Dummy Session";
-
     case SESSION_STATE_READY:
         return "Session Ready";
 
@@ -765,7 +715,7 @@ MXS_SESSION* session_get_ref(MXS_SESSION* session)
 
 void session_put_ref(MXS_SESSION* session)
 {
-    if (session && session->state != SESSION_STATE_DUMMY)
+    if (session)
     {
         /** Remove one reference. If there are no references left, free session */
         if (mxb::atomic::add(&session->refcount, -1) == 1)
@@ -904,7 +854,6 @@ void session_qualify_for_pool(MXS_SESSION* session)
 
 bool session_valid_for_pool(const MXS_SESSION* session)
 {
-    mxb_assert(session->state != SESSION_STATE_DUMMY);
     return session->qualifies_for_pooling;
 }
 
@@ -1148,7 +1097,8 @@ const char* session_get_close_reason(const MXS_SESSION* session)
     }
 }
 
-Session::Session(SERVICE* service)
+Session::Session(SERVICE* service, DCB* client_dcb)
+    : MXS_SESSION(client_dcb)
 {
     if (service->retain_last_statements != -1) // Explicitly set for the service
     {

server/modules/filter/test/mock_session.cc

@@ -27,13 +27,13 @@ namespace mock
 {
 
 Session::Session(Client* pClient)
-    : mxs::Session(&dummy_service)
+    : mxs::Session(&dummy_service, &m_client_dcb)
     , m_client(*pClient)
     , m_client_dcb(this, pClient->user(), pClient->host(), pClient)
 {
     MXS_SESSION* pSession = this;
 
-    memset(pSession, 0, sizeof(MXS_SESSION));
+    memset((void*)pSession, 0, sizeof(MXS_SESSION));
 
     pSession->state = SESSION_STATE_ALLOC;
 

server/modules/protocol/CDC/cdc.cc

@@ -293,9 +293,6 @@ static int cdc_accept(DCB* client_dcb)
 
     client_dcb->protocol = (CDC_protocol*) protocol;
 
-    /* Dummy session */
-    client_dcb->session = session_set_dummy(client_dcb);
-
     if (NULL == client_dcb->session || poll_add_dcb(client_dcb))
     {
         dcb_close(client_dcb);

server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc

@@ -347,11 +347,8 @@ static void handle_error_response(DCB* dcb, GWBUF* buffer)
              || errcode == ER_DBACCESS_DENIED_ERROR
              || errcode == ER_ACCESS_DENIED_NO_PASSWORD_ERROR)
     {
-        if (dcb->session->state != SESSION_STATE_DUMMY)
+        // Authentication failed, reload users
-        {
+        service_refresh_users(dcb->service);
-            // Authentication failed, reload users
-            service_refresh_users(dcb->service);
-        }
     }
 }
 
@@ -404,12 +401,8 @@ static inline void prepare_for_write(DCB* dcb, GWBUF* buffer)
 {
     MySQLProtocol* proto = (MySQLProtocol*)dcb->protocol;
 
-    /**
+     // The DCB's session is set to null when it is put into the persistent connection pool.
-     * The DCB's session is set to the dummy session when it is put into the
+    if (dcb->session)
-     * persistent connection pool. If this is not the dummy session, track
-     * the current command being executed.
-     */
-    if (!session_is_dummy(dcb->session))
     {
         uint64_t capabilities = service_get_capabilities(dcb->session->service);
 
@@ -486,11 +479,7 @@ static int gw_read_backend_event(DCB* dcb)
         return 0;
     }
 
-    if (dcb->session == NULL
+    mxb_assert(dcb->session);
-        || dcb->session->state == SESSION_STATE_DUMMY)
-    {
-        return 0;
-    }
 
     MySQLProtocol* proto = (MySQLProtocol*)dcb->protocol;
 
@@ -1299,7 +1288,7 @@ static int gw_error_backend_event(DCB* dcb)
 {
     MXS_SESSION* session = dcb->session;
 
-    if (session->state == SESSION_STATE_DUMMY)
+    if (!session)
     {
         if (dcb->persistentstart == 0)
         {

server/modules/protocol/MySQL/mariadbclient/mysql_client.cc

@@ -774,8 +774,7 @@ static int gw_read_do_authentication(DCB* dcb, GWBUF* read_buffer, int nbytes_re
          */
         if (session_start(dcb->session))
         {
-            mxb_assert(dcb->session->state != SESSION_STATE_ALLOC
+            mxb_assert(dcb->session->state != SESSION_STATE_ALLOC);
-                       && dcb->session->state != SESSION_STATE_DUMMY);
             // For the time being only the sql_mode is stored in MXS_SESSION::client_protocol_data.
             dcb->session->client_protocol_data = QC_SQL_MODE_DEFAULT;
             protocol->protocol_auth_state = MXS_AUTH_STATE_COMPLETE;
@@ -1464,8 +1463,7 @@ static int gw_client_close(DCB* dcb)
     {
         MXS_SESSION* target = dcb->session;
 
-        if (target->state != SESSION_STATE_TO_BE_FREED
+        if (target->state != SESSION_STATE_TO_BE_FREED)
-            && target->state != SESSION_STATE_DUMMY)
         {
             mxb_assert(target->state == SESSION_STATE_ROUTER_READY
                        || target->state == SESSION_STATE_STOPPING);
@@ -1492,7 +1490,7 @@ static int gw_client_hangup_event(DCB* dcb)
 
     if (session)
     {
-        if (session->state != SESSION_STATE_DUMMY && !session_valid_for_pool(session))
+        if (!session_valid_for_pool(session))
         {
             if (session_get_dump_statements() == SESSION_DUMP_STATEMENTS_ON_ERROR)
             {

server/modules/protocol/MySQL/mysql_common.cc

@@ -596,8 +596,7 @@ bool gw_get_shared_session_auth_info(DCB* dcb, MYSQL_session* session)
         mxb_assert(dcb->data);
         memcpy(session, dcb->data, sizeof(MYSQL_session));
     }
-    else if (dcb->session->state != SESSION_STATE_ALLOC
+    else if (dcb->session->state != SESSION_STATE_ALLOC)
-             && dcb->session->state != SESSION_STATE_DUMMY)
     {
         memcpy(session, dcb->session->client_dcb->data, sizeof(MYSQL_session));
     }