[improve] Support client max idle connections minutes

[wenbingshen]

Motivation

Related to this PR #3913, I noticed that the bookkeeper client often needs to maintain a permanent long connection with the bookie server that has established a connection. I guess the reason is that the creation of ensemble is based on the polling selection of all bookie nodes. Long-running services are bound to Keep in touch with all bookie servers repeatedly.

Permanent long connections, especially when the client and server do not have any read-write operations, are a waste of resources (such as server-side file handles, resources occupied by socket creation internal objects, possible memory leaks).

When the AutoRecovery service runs for a long time, its detection task will generally occur once every 1 day, 7 days, or even longer. Once the connection is established with all bookie servers, it will not actively release the connection unless some abnormality occurs.

Changes

For this reason, I want to add a maximum idle connection time on the client side to actively close some network connections that have not performed read-write operations for a long time.
This future is disabled by default.
New configuration: clientMaxIdleConnectionsMinutes, Default is -1 (disabled)

[eolivelli]

I have left some comments.
I am not sure if this is adding cost on the hot paths

can you please also add tests ?

[eolivelli]

why minutes and not "seconds" ?

[wenbingshen]

seconds should be more generic, I'll address it.

[eolivelli]

how much cost adds "asMap" ? is performing operations or is it no-op ?
if it is no-op then we could cache the result of "asMap" into a field.
if it performs operations, then we are adding work on the hot paths (anytime we read/write) and we should find a better way to expire idle connections, maybe not using a "Cache" but with a specific background task that scans the map and removes idle connection

[wenbingshen]

how much cost adds "asMap" ? is performing operations or is it no-op ? if it is no-op then we could cache the result of "asMap" into a field. if it performs operations, then we are adding work on the hot paths (anytime we read/write) and we should find a better way to expire idle connections, maybe not using a "Cache" but with a specific background task that scans the map and removes idle connection

Here I made a mistake, we can't use asMap, it won't update the latest access time of the key.
it should be replaced with the following code:

                PerChannelBookieClientPool oldClientPool = channels.getIfPresent(addr);        <= here a1
                if (null == oldClientPool) {
                    channels.put(addr, newClientPool);         <== here a2
                    clientPool = newClientPool;
                    // initialize the pool only after we put the pool into the map
                    clientPool.initialize();
                }

[wenbingshen]

I have left some comments. I am not sure if this is adding cost on the hot paths

IMO, the code below is the core hot path, active connections will get hits in channels most of the time:

    public PerChannelBookieClientPool lookupClient(BookieId addr) {
        PerChannelBookieClientPool clientPool = channels.getIfPresent(addr);
        return clientPool;
    }

A new connection will only be created when the bookie node is expanded or restarted.

can you please also add tests ?

I will add tests.

[horizonzy]

We would better add tests to verify the feature.

[StevenLuMT]

yes,We would better add tests to verify the feature.

[StevenLuMT]

max idle connections per second?