Good idea. I has modify the implementation:
/** Create time. **/
private final long createTime;
/** The time when marks the connection is idle. **/
private long IdleMarkTime;
/** The time when the last valid data was transmitted. **/
private long lastWorkTime;
/** Stat **/
private State stat;
/**
* Check client connection is now free. This method may change the state
to idle.
* This method will not change the state to idle.
*/
public boolean doIdleCheck();
/** Get stat **/
public State getStat();
/** Change stat **/
public boolean setStat(State originalStat, State newStat);
public enum State {
/** The connection is in use. **/
using,
/** The connection is not in use. **/
idle_marked,
/** The connection will be released soon. **/
before_release,
/** The connection has already been released. **/
released;
}
On Mon, Jun 6, 2022 at 9:32 AM 一苇以航 <[email protected]> wrote:
> Hi yubiao
> This is a good idea. But I have a question about the implementation.
> I noticed that you use an int variable 'stat' to identify the stat of
> connection. But a more general approach in Pulsar is to define a new stats
> class that has an enum 'State' and some state-related method. And then make
> the class extend the new class. Is there any problem with using this
> implementation?
> Xiangying Meng
> Thanks
>
>
> ------------------ 原始邮件 ------------------
> 发件人:
> "dev"
> <
> [email protected]>;
> 发送时间: 2022年6月6日(星期一) 凌晨1:08
> 收件人: "dev"<[email protected]>;
>
> 主题: Re: [DISCUSS] [PIP-165] Auto release client useless connections
>
>
>
> Hi Ran
>
> I think you mean that: Producer/Consumer failed to establish a connection
> when he tried to work again.
>
> There are two places in the Broker configuration that limit the maximum
> number of connections:
> - Broker config : maxConnectionsLimitEnabled
> - Broker config: maxConnectionsLimitPerIpEnabled
>
> At client side:
> We only release connections that are not registered with producer or
> Consumer or Transaction. So when a new producer creates it will get an
> error (NotAllowedError because reached the maximum number of
> connections) same as original design.
>
> At proxy side:
> I'm sorry I didn't think it through before. I changed the proxy part of the
> proposal:
>
> The connection between proxy and broker has two parts: For lookup commands;
> For consumers, producers commands and other commands.
> The connection "For consumers, producers commands and other
> commands" is
> managed by DirectProxyHandler, which holds the connection until the client
> is closed, so it does not affect of producers or consumers, These
> connections do not require additional closing.
> The connection "For lookup commands": When the proxy is configured
> `metadataStoreUrl`, the Lookup Command will select the registered broker by
> rotation training and create a connection. If we do not optimize the broker
> load balancing algorithm, all connections are considered useful
> connections.
> When the cluster is large, holds so many connections becomes redundant.
> Later, I will try to put forward other proposals to improve this
> phenomenon, so this proposal does not involve proxy connection release.
>
>
>
>
> On Fri, Jun 3, 2022 at 11:44 AM Ran Gao <[email protected]> wrote:
>
> > This is a good idea, but I have a concern, Pulsar has the config
> > `brokerMaxConnections` to control max connection count against one
> broker.
> > If the connection is closed, it will re-connect when consumers or
> producers
> > start to consume and produce messages again, but this time the max
> > connection count will reach the max count.
> >
> >
> > On 2022/05/26 06:31:37 Yubiao Feng wrote:
> > > I open a pip to discuss Auto release client useless connections,
> could
> > you
> > > help me review
> > >
> > >
> > > ## Motivation
> > > Currently, the Pulsar client keeps the connection even if no
> producers or
> > > consumers use this connection.
> > > If a client produces messages to topic A and we have 3 brokers
> 1, 2, 3.
> > Due
> > > to the bundle unloading(load manager)
> > > topic ownership will change from A to B and finally to C. For
> now, the
> > > client-side will keep 3 connections to all 3 brokers.
> > > We can optimize this part to reduce the broker side connections,
> the
> > client
> > > should close the unused connections.
> > >
> > > So a mechanism needs to be added to release unwanted connections.
> > >
> > > ### Why are there idle connections?
> > >
> > > 1.When configuration `maxConnectionsPerHosts ` is not set to 0,
> the
> > > connection is not closed at all.
> > > The design is to hold a fixed number of connections per Host,
> avoiding
> > > frequent closing and creation.
> > >
> > >
> >
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L325-L335
> >
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L325-L335>>;
> >
> > > 2-1. When clients receive `command-close`, will reconnect
> immediately.
> > > It's designed to make it possible to reconnect, rebalance, and
> unload.
> > >
> > >
> >
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141
> >
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141>>;
> >
> > > 2-2. The broker will close client connections before writing
> ownership
> > info
> > > to the ZK. Then clients will get deprecated broker address when
> it tries
> > > lookup.
> > >
> > >
> >
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1282-L1293
> >
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1282-L1293>>;
> >
> > > ## Goal
> > > Automatically release connections that are no longer used.
> > >
> > > - Scope
> > > - **Pulsar client**
> > > Contains connections used by consumers, Producers, and
> Transactions.
> > >
> > > - **Pulsar proxy**
> > > Contains only the connection between Proxy and broker
> > >
> > > ## Approach
> > > Periodically check for idle connections and close them.
> > >
> > > ## Changes
> > >
> > > ### API changes
> > > **ClientCnx** added an idle check method to mark idle time.
> > >
> > > ```java
> > > /** Create time. **/
> > > private final long createTime;
> > > /** The time when marks the connection is idle. **/
> > > private long IdleMarkTime;
> > > /** The time when the last valid data was transmitted. **/
> > > private long lastWorkTime;
> > > /** Stat. enumerated values: using, idle_marked, before_release,
> > released**/
> > > private int stat;
> > > /**
> > > * Check client connection is now free. This method
> may change the state
> > > to idle.
> > > * This method will not change the state to idle.
> > > */
> > > public boolen doIdleCheck();
> > > /** Get stat **/
> > > public int getStat();
> > > /** Change stat **/
> > > public int setStat(int originalStat, int newStat);
> > > ```
> > >
> > > ### Configuration changes
> > > We can set the check frequency and release rule for idle
> connections at
> > > `ClientConfigurationData`.
> > >
> > > ```java
> > > @ApiModelProperty(
> > > name =
> "autoReleaseIdleConnectionsEnabled",
> > > value = "Do you
> want to automatically clean up unused
> > connections"
> > > )
> > > private boolean autoReleaseIdleConnectionsEnabled = true;
> > >
> > > @ApiModelProperty(
> > > name =
> "connectionMaxIdleSeconds",
> > > value = "Release
> the connection if it is not used for more than
> > > [connectionMaxIdleSeconds] seconds"
> > > )
> > > private int connectionMaxIdleSeconds = 180;
> > >
> > > @ApiModelProperty(
> > > name =
> "connectionIdleDetectionIntervalSeconds",
> > > value = "How
> often check idle connections"
> > > )
> > > private int connectionIdleDetectionIntervalSeconds = 60;
> > > ```
> > >
> > > ## Implementation
> > >
> > > - **Pulsar client**
> > > If no consumer, producer, or transaction uses the current
> connection,
> > > release it.
> > >
> > > - **Pulsar proxy**
> > > If the connection has not transmitted valid data for a long
> time, release
> > > it.
> > >
> > >
> > > Yubiao Feng
> > > Thanks
> > >
> >
