From 8159fe07ed5a46148fa9d46b3b4efb590259d729 Mon Sep 17 00:00:00 2001 From: Oknet Xu Date: Fri, 1 Sep 2017 22:18:54 +0800 Subject: [PATCH] Optimize: If failed on migrateToCurrentThread, put the server session back to global server session pool --- iocore/net/UnixNetVConnection.cc | 74 ++++++++++++++++++++++---------- proxy/http/HttpSessionManager.cc | 24 +++++------ 2 files changed, 61 insertions(+), 37 deletions(-) diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc index 63d5ba0b73d..fc613037d40 100644 --- a/iocore/net/UnixNetVConnection.cc +++ b/iocore/net/UnixNetVConnection.cc @@ -1244,6 +1244,7 @@ UnixNetVConnection::populate(Connection &con_in, Continuation *c, void *arg) } if (h->startIO(this) < 0) { + con_in.move(this->con); Debug("iocore_net", "populate : Failed to add to epoll list"); return EVENT_ERROR; } @@ -1430,44 +1431,71 @@ UnixNetVConnection::migrateToCurrentThread(Continuation *cont, EThread *t) return this; } - Connection hold_con; - hold_con.move(this->con); - SSLNetVConnection *sslvc = dynamic_cast(this); - - SSL *save_ssl = (sslvc) ? sslvc->ssl : nullptr; - if (save_ssl) { - SSLNetVCDetach(sslvc->ssl); - sslvc->ssl = nullptr; + // Lock the NetHandler first in order to put the new NetVC into NetHandler and InactivityCop. + // It is safe and no performance issue to get the mutex lock for a NetHandler of current ethread. + SCOPED_MUTEX_LOCK(lock, client_nh->mutex, t); + + // Try to get the mutex lock for NetHandler of this NetVC + MUTEX_TRY_LOCK(lock_src, this->nh->mutex, t); + if (lock_src.is_locked()) { + // Deattach this NetVC from original NetHandler & InactivityCop. + this->nh->stopCop(this); + this->nh->stopIO(this); + // Put this NetVC into current NetHandler & InactivityCop. + this->thread = t; + client_nh->startIO(this); + client_nh->startCop(this); + // Move this NetVC to current EThread Successfully. + return this; } - // Do_io_close will signal the VC to be freed on the original thread - // Since we moved the con context, the fd will not be closed - // Go ahead and remove the fd from the original thread's epoll structure, so it is not - // processed on two threads simultaneously - this->ep.stop(); - this->do_io_close(); + // Failed to get the mutex lock for original NetHandler. + // Try to migrate it by create a new NetVC and then move con.fd and ssl ctx. + SSLNetVConnection *sslvc = dynamic_cast(this); + SSL *save_ssl = (sslvc) ? sslvc->ssl : nullptr; + + UnixNetVConnection *ret_vc = nullptr; // Create new VC: if (save_ssl) { SSLNetVConnection *sslvc = static_cast(sslNetProcessor.allocate_vc(t)); - if (sslvc->populate(hold_con, cont, save_ssl) != EVENT_DONE) { - sslvc->do_io_close(); - sslvc = nullptr; + if (sslvc->populate(this->con, cont, save_ssl) != EVENT_DONE) { + sslvc->free(t); + sslvc = nullptr; + ret_vc = this; } else { sslvc->set_context(get_context()); + ret_vc = dynamic_cast(sslvc); } - return sslvc; - // Update the SSL fields } else { UnixNetVConnection *netvc = static_cast(netProcessor.allocate_vc(t)); - if (netvc->populate(hold_con, cont, save_ssl) != EVENT_DONE) { - netvc->do_io_close(); - netvc = nullptr; + if (netvc->populate(this->con, cont, save_ssl) != EVENT_DONE) { + netvc->free(t); + netvc = nullptr; + ret_vc = this; } else { netvc->set_context(get_context()); + ret_vc = netvc; } - return netvc; } + + // clear con.fd and ssl ctx from this NetVC since a new NetVC is created. + if (ret_vc != this) { + if (save_ssl) { + SSLNetVCDetach(sslvc->ssl); + sslvc->ssl = nullptr; + } + ink_assert(this->con.fd == NO_FD); + + // Do_io_close will signal the VC to be freed on the original thread + // Since we moved the con context, the fd will not be closed + // Go ahead and remove the fd from the original thread's epoll structure, so it is not + // processed on two threads simultaneously + this->ep.stop(); + this->do_io_close(); + } + + return ret_vc; } void diff --git a/proxy/http/HttpSessionManager.cc b/proxy/http/HttpSessionManager.cc index 1e4334a2336..49450f2dc8e 100644 --- a/proxy/http/HttpSessionManager.cc +++ b/proxy/http/HttpSessionManager.cc @@ -326,21 +326,17 @@ HttpSessionManager::acquire_session(Continuation * /* cont ATS_UNUSED */, sockad UnixNetVConnection *server_vc = dynamic_cast(to_return->get_netvc()); if (server_vc) { UnixNetVConnection *new_vc = server_vc->migrateToCurrentThread(sm, ethread); - // The VC moved, free up the original one - if (new_vc != server_vc) { - ink_assert(new_vc == nullptr || new_vc->nh != nullptr); - if (!new_vc) { - // Close out to_return, we were't able to get a connection - to_return->do_io_close(); - to_return = nullptr; - retval = HSM_NOT_FOUND; - } else { - // Keep things from timing out on us - new_vc->set_inactivity_timeout(new_vc->get_inactivity_timeout()); - to_return->set_netvc(new_vc); - } + if (new_vc->thread != ethread) { + // Failed to migrate, put it back to global session pool + m_g_pool->releaseSession(to_return); + to_return = nullptr; + retval = HSM_NOT_FOUND; + } else if (new_vc != server_vc) { + // The VC migrated, keep things from timing out on us + new_vc->set_inactivity_timeout(new_vc->get_inactivity_timeout()); + to_return->set_netvc(new_vc); } else { - // Keep things from timing out on us + // The VC moved, keep things from timing out on us server_vc->set_inactivity_timeout(server_vc->get_inactivity_timeout()); } }