Transferring Sessions between threads for Http Client Connection Pool

[mikekliger]

Hi! Currently I have a homemade threadLocal Connection pool class that retrieves sessions via an array of "Connector managers" which implements HttpConnector Callback. When we call connect() the HTTP::Connector CallBack connectSuccess() returns a session which then has an idle timeout specified by a wangle::ConnectionManager. We keep pointers to these sessions so we can age them out according to our own maxAge property too.

We are trying to migrate to proxygen::SessionPool though to take advantage of transferring idle sessions. I am wondering if it is necessary to refactor our session and connection creation as well? I see in proxygen/connpool/test there is a SessionPoolTestFixture that allows sessions to be created without an HttpConnector Callback, but i assume its just for testing.

Basically, I am confused if I have a connectionManager on one thread working in parallel to manage connections/sessions that are managed by a SessionPool, when i transfer it to another thread, I believe it is not possible to transfer the other thread's connection manager and thus two objects (on two different threads) will be accessing the same connection. Is the connection Manager maybe supposed to be at a higher level and have interaction with all of our session pools?

[afrind]

The ServerIdleSessionController should detach the session from the connection manager by calling detachThreadLocals: https://github.com/facebook/proxygen/blob/main/proxygen/lib/http/connpool/ServerIdleSessionController.cpp#L38.

Looks like there's a little but of manual work required, some of our non-oss code has this bit:

      auto future = serverIdleSessionController_.getIdleSession();
      auto func = [=](HTTPSessionBase* session) {
        HTTPTransaction* txn = nullptr;
        if (session) {
          auto evb = session->getEventBase();
          if (!evb || !evb->isInEventBaseThread()) {
            session->attachThreadLocals(
                eventBase, ...);
            auto cm = pool_->getTLUpstreamConnManager(eventBase);
            if (cm) {
              cm->addConnection(session, true);
            }
          }
        }
      };
      return std::move(future)
          .via(eventBase)
          .thenValue(func);

[mikekliger]

I see! thanks. If i acquire a session via HTTP Connector Callback, and call putSession() to move it into my pool, does the pool override the previously set idleTimeout as soon as the session is put into the pool (Which would come from the wangle::ConnectionManager)

[afrind]

The ConnectionManager controls the idle timeout (when the session has no open transactions). But if you set a transaction timeout on the session at creation time, that should stick when adding to a different CM.

[mikekliger]

Thank you that is very helpful. I am still running into a memory issue in unit testing, but i believe it may be an issue in how we are managing our evb/threads.

On issue 314 you mention that thread transferring can be a huge memory win from server POV, , do you observe the same magnitude of effect on client side?

[afrind]

For us the memory win on the "client" side (eg: the upstream half of the proxy) is the significant bit. The total memory savings should be roughly the same on both sides, but there are fewer proxies than servers and they tend to be more memory constrained.

[mikekliger]

Thanks so much, thats great to hear.

I have one last question before closing: between calling attachThreadLocals() and getIdleSession(), where does the HTTPConnector come into play? Is there a relationship between the session's connection that is being transferred, and its HTTPConnector that is used to establish that connection?

I know in connectionManager I have access to a callback called onConnectionRemoved(const wangle::ManagedConnection* c). Once I have a connection/session established, what role does the HttpConnector play? If i transfer a connection/session I would think I need to either transfer this HttpConnector along with them, or return it to the pool of connectors we use along with our connection manager (since this is how we obtain new sessions/connections).

[mikekliger]

So thread A gets session from Thread B. Thread B removes connection from Thread B's connection manager. From thread B perspective, I would need to free up that HTTP connector. But unless I am missing something, onConnectionRemoved() does not give me that access

[mikekliger]

Ah I made a mistake. I suppose the connector is irrelevant and it will free itself upon connectSuccess or Error. i will close the issue today.