Total connections created exceeding expected count for fix amount of requests

[prathamk]

Jetty version
9.4.24.v20191120
Java version
1.8 b251
OS type/version
linux (RHEL 7.7)
Description
Why the number of connections to the server are exceeding the expected number of connections?

We are trying to create a round-robin connection pool, with max-streams support which is configurable. When the max-streams of the connection are exhausted, the connection is reset, and a new connection is created in its place.
But the behavior is inconsistent, and newer connections are created, before the max streams of these connections are exhausted.

For example:
Max Connections: 3
Max Streams per connection: 10
We are send 30 requests, we need these 3 connections to serve all the requests(3 * 10 = 30) in round robin manner, and no new connection should be created.
Upon receiving the 31st request, and new connection should be created, as the older connection's streams have been exhausted. Same for 32nd and 33rd request.
But the behavior is inconsistent, and newer connections are created, even while serving these 30 requests.

Please find the file attached for the same(zipped):
RoundRobinConnectionPoolWithStreamId.zip

[sbordet]

This has been addressed and fixed in #4904 which is available in Jetty 9.4.30.
Can you please try the latest version of Jetty?

[prathamk]

Hi,

Tried with the latest version (9.4.30.v20200611) also, still the same issue.

As we are trying to create a round-robin connection pool, with configurable stream IDs. The streams of a connection gets exhausted on reaching the value of max streams per connection.

So we have extended the class AbstractConnectionPool (also implements Multiplexable) and created our connection pool by overriding some of the methods.
As a part of it, we also modified the activate() method. We are able to account for the times when any request comes, and the thread enters into the activate() method for connection. But not able to figure out, why it is coming more number of times, than the connections required to serve the requests?

The remove() and release() method, implementations can also be found in the file shared earlier.

Our implemention of activate() method with stream ID exhaustion:

protected Connection activate()
    {
        System.out.println("************ Activate **************");
        Connection connection = null;
        lock();
        try
        {
            int offset = 0;
            int capacity = getMaxConnectionCount();
            while (offset < capacity)
            {
                int idx = index + offset;
                if (idx >= capacity)
                    idx -= capacity;
                Entry entry = entries.get(idx);
                if (entry.connection == null)
                    break;
                if (entry.active < getMaxMultiplex())
                {
                    ++entry.active;
                    ++entry.used;
                    connection = entry.connection;
                    System.out.println("Request Count --> " + requestCount.incrementAndGet());
                    if (entry.used == maxStreamsPerConnection)
                    {
                        Entry tmpEntry = new Entry();
                        tmpEntry.active = entry.active;
                        tmpEntry.used = entry.used;
                        expiredConnections.put(connection, tmpEntry);
                        entry.reset();
                    }

                    index += offset + 1;
                    if (index >= capacity)
                        index -= capacity;
                    break;
                }
                ++offset;
            }
        }
        finally
        {
            unlock();
        }
        System.out.println("************ end Activate **************" + connection);
        return connection == null ? null : active(connection);
    }

Example(as the scenario run in log):
For example:
Max Connections: 5
Max Streams per connection: 10
We are send 50 requests, we need these 5 connections to serve all the requests(5 * 10 = 50) in round robin manner, and no new connection should be created.
Upon receiving the 51st request, and new connection should be created, as the older connection's streams have been exhausted. Same for 52nd and 53rd request and so on.
But the behavior is inconsistent, and newer connections are created, even while serving these 50 requests.

Please find attched logs for the same.
Log.txt

Please suggest, is this an issue? or are we missing something?

[prathamk]

hi,

As part of our ongoing investigation, we had below observation:

Whenever we hit a request, the request lands on the HttpDestination class. In this class a private method process() is invoked from the send() method, which gets the connection pool and on that connection process that request.

With the existing implementation, we are getting the connections used count getting incremented more number of times than the expected count(Total requests hit).

But if we change the existing implementation of the send() method (make it synchronized), we are getting the expected result.

Existing code:
Existing implementation in HttpDestination class.

public void send()
    {
        if (getHttpExchanges().isEmpty())
            return;
        process();
    }

Modified Code:
Created new class by extending the class HttpDestinationOverHTTP2 which further down the line extends HttpDestination class. And overridden the send() method.

@Override
    public void send()
    {
        synchronized(this)
        {
            super.send();
        }
    }

Though we have an impact on tps with this change, but from functionality point it is giving desired result.
What might be the issue in existing implementation? Or are we missing something?

[sbordet]

If you have maxConnectionsPerDestination=5, then there will never be more than 5 connections created for that destination.
Therefore, I don't understand why you expect a new connection to be created on the 51st request?

Using a lock per destination is not something we will do - it will kill the performance.
However, if it works for your custom connection pool, go for it.

In summary, I don't understand what you want.
Seems to me that you want a TCP connection to be used for N times, where N is the multiplex value, and then not anymore.
This will be very inefficient, so you want to expand on why you want to do that.

If this is still the behavior you want, it's not currently implemented and you have to write your own connection pool.
It will be tricky to find the right moment to close the connection though - you probably have to quarantine the connection until all the multiplexed requests are completed.

The key thing to understand is that both the sending thread and the receiving thread compete to send queued requests.
Your implementation should take this behavior into account.
We do this in the current implementations by calling acquire(boolean) where the boolean parameter decides whether to create a new connection or not when no connections are available.

The only exception is RoundRobinConnectionPool: by its nature (i.e. trying to always use a different connection and aggressively create new connections if possible), it is possible that more connections than needed are created, but they will be idle timed out if they are not used, so typically it's not an issue.

[prathamk]

Hello sbordet,

Thanks for your response.

Basically our requirement is to configure maximum streams per connection. In the current implementation, each connection serve Integer.Max_Value streams. we need to configure these stream id's per connection, so when each connection completed configured number of stream, it will set for Idle Timeout and new connection would be created to maintain maximum number of connection.

We have created our own Round Robin connection pool where our connection pool maintain the maximum number of stream per connection but unfortunately since multiple thread are calling HttpDestination.send method, so sometime connection is returned from acquired method of connection pool but not used for sending the request. This means, if we send 25 request in connection there is possibility of acquired method will be called more than 25 times.

We understand that in case multiplexing, request thread and response thread should quarantine the connection until all request are completed. We have done similar changes as we are making connection idle for some time before closing the connection so that are request are completed.

We only need to configure maximum stream per connection and want to close connection if maximum stream is reached and create a new connection.

We already provided our connection pool implementation above.

Kindly suggest the way forward are we missing anything or there is other way to implement this functionality.??

[sbordet]

I think this is the same as #4809 then.

Does your implementation work after the changes?

[prathamk]

Hello sbordet,

We are able to close the connection, when the max stream per connection are consumed, by making the connection idle timeout, and not getting the exception AsynchronousCloseException.
But not able to workout the scenario as mentioned above (unless make the send() method synchronized). The count of streams consumed of a connection is getting increased more number of times than the requests it has received.
We suspect the threads call the acquire method, but doesn't serve the request on every connection acquired, as this part is not synchronized, and there may be a chance that multiple threads check queue httpexchanges for isEmpty, and then call the acquire method. Then again in process method, there is a check for httpExchange non null, but by that time that thread has already acquired the connection (and in our case the stream count has been incremented without serving request).

Kindly suggest the way forward are we missing anything or there is other way to implement this functionality.??

[sbordet]

We fixed #4904 in 9.4.31 (sorry, above I said 9.4.30, but it's in 9.4.31).
We have implemented AbstactConnectionPool.acquire(boolean) that should help in your case.
Give the latest code a try.

[prathamk]

The JARs for version 9.4.31, are not yet available in maven repo. Where can we get the latest JARs?

[sbordet]

You can build Jetty yourself (easy, see https://www.eclipse.org/jetty/documentation/current/contributing-source-build.html and build skipping the tests: mvn clean install -DskipTests).

Or you can reference this repository in your POM:
https://oss.sonatype.org/content/groups/jetty-with-staging

It currently contains version 9.4.31 that has been staged, we are testing it, and we will release it in a day or so.

Or you can wait a day or so for 9.4.31 to appear in Maven Central.

[prathamk]

Hello sbordet,

We tried with version 9.4.31 also, but still having the issue.

In the latest code also, the acquire() method is called with the boolean create variable. In this case the connection will only be created when the connection is null and create is true.

protected Connection acquire(boolean create)
    {
        Connection connection = activate();
        if (connection == null && create)
        {
            tryCreate(destination.getQueuedRequestCount());
            connection = activate();
        }
        return connection;
    }

But the acquire() method is still called more number of times, which in turn calls activate() method.
The code snippet from logic in activate() method:

++entry.active;
++entry.used;
connection = entry.connection;
if (entry.used == maxStreamsPerConnection)
{
   //Set current connection to null, so it will be created on next request hit in round robin manner.
}

So in our scenario, there might be some threads which have got the connection (incremented the streams used count), but didn't serve any requests, and this would in turn close the connection before utilizing all streams (equal to max streams per connection).

There might be a possibility that the active count of connection might not reach to 0 due to this same behavior while releasing the connection and hence it may not be marked idle.

We are setting the max Connection per destination and max Streams per connection accordingly.

[sbordet]

The design of HttpClient is such that the sender and the receiver threads concurrently try to send queued requests.

This is done for necessity (if the sender thread stops sending, queued requests need to be sent by the receiver thread) as well as performance. We are not going to change this.

A ConnectionPool pools connections and applications should not be much concerned about how this is done (do you know how many your browser opens?), or whether an extra (or two) connections are being created - they will be used eventually, thus improving performance, or will idle time out.

Commercial load balancers are not perfect either, they may send requests to 2 servers in a non perfect alternance.

You have not explained why you really need to be that strict on your logic.
Why can you not tolerate one extra connection?

Having said that, you have found the solution of synchronizing send() so if that works for you, go with it.

[sbordet]

@prathamk one of your requirements, max streams per connection - or more in general, max usage per connection, has been implemented in #4809.

Can you please try the latest code, and set your desired max usage count?

[sbordet]

Closing as resolved.