You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Ran Gao <rg...@apache.org> on 2022/06/03 03:44:22 UTC

Re: [DISCUSS] [PIP-165] Auto release client useless connections

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
> 
> 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
> 
> 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
> 
> ## 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
> 

回复: [DISCUSS] [PIP-165] Auto release client useless connections

Posted by 一苇以航 <19...@qq.com.INVALID>.
Good work.
+1




------------------&nbsp;原始邮件&nbsp;------------------
发件人:                                                                                                                        "dev"                                                                                    <rgao@apache.org&gt;;
发送时间:&nbsp;2022年6月7日(星期二) 上午9:54
收件人:&nbsp;"dev"<dev@pulsar.apache.org&gt;;

主题:&nbsp;Re: [DISCUSS] [PIP-165] Auto release client useless connections



Ok, thanks for your explanation, make sense to me.

+1

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

Re: [DISCUSS] [PIP-165] Auto release client useless connections

Posted by Ran Gao <rg...@apache.org>.
Ok, thanks for your explanation, make sense to me.

+1

On 2022/06/05 17:08:22 Yubiao Feng wrote:
> 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 <rg...@apache.org> 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
> > >
> > > 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
> > >
> > > 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
> > >
> > > ## 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
> > >
> >
> 

Re: [DISCUSS] [PIP-165] Auto release client useless connections

Posted by Yubiao Feng <yu...@streamnative.io.INVALID>.
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 一苇以航 <19...@qq.com.invalid> 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
>
>
> ------------------&nbsp;原始邮件&nbsp;------------------
> 发件人:
>                                                   "dev"
>                                                                 <
> yubiao.feng@streamnative.io.INVALID&gt;;
> 发送时间:&nbsp;2022年6月6日(星期一) 凌晨1:08
> 收件人:&nbsp;"dev"<dev@pulsar.apache.org&gt;;
>
> 主题:&nbsp;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)&nbsp; 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&nbsp; and other commands.
> The connection "For consumers, producers commands&nbsp; 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 <rgao@apache.org&gt; wrote:
>
> &gt; This is a good idea, but I have a concern, Pulsar has the config
> &gt; `brokerMaxConnections` to control max connection count against one
> broker.
> &gt; If the connection is closed, it will re-connect when consumers or
> producers
> &gt; start to consume and produce messages again, but this time the max
> &gt; connection count will reach the max count.
> &gt;
> &gt;
> &gt; On 2022/05/26 06:31:37 Yubiao Feng wrote:
> &gt; &gt; I open a pip to discuss Auto release client useless connections,
> could
> &gt; you
> &gt; &gt; help me review
> &gt; &gt;
> &gt; &gt;
> &gt; &gt; ## Motivation
> &gt; &gt; Currently, the Pulsar client keeps the connection even if no
> producers or
> &gt; &gt; consumers use this connection.
> &gt; &gt; If a client produces messages to topic A and we have 3 brokers
> 1, 2, 3.
> &gt; Due
> &gt; &gt; to the bundle unloading(load manager)
> &gt; &gt; topic ownership will change from A to B and finally to C. For
> now, the
> &gt; &gt; client-side will keep 3 connections to all 3 brokers.
> &gt; &gt; We can optimize this part to reduce the broker side connections,
> the
> &gt; client
> &gt; &gt; should close the unused connections.
> &gt; &gt;
> &gt; &gt; So a mechanism needs to be added to release unwanted connections.
> &gt; &gt;
> &gt; &gt; ### Why are there idle connections?
> &gt; &gt;
> &gt; &gt; 1.When configuration `maxConnectionsPerHosts ` is not set to 0,
> the
> &gt; &gt; connection is not closed at all.
> &gt; &gt; The design is to hold a fixed number of connections per Host,
> avoiding
> &gt; &gt; frequent closing and creation.
> &gt; &gt;
> &gt; &gt;
> &gt;
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L325-L335
> &gt
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L325-L335&gt>;
> &gt;
> &gt; &gt; 2-1. When clients receive `command-close`, will reconnect
> immediately.
> &gt; &gt; It's designed to make it possible to reconnect, rebalance, and
> unload.
> &gt; &gt;
> &gt; &gt;
> &gt;
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141
> &gt
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141&gt>;
> &gt;
> &gt; &gt; 2-2. The broker will close client connections before writing
> ownership
> &gt; info
> &gt; &gt; to the ZK. Then clients will get deprecated broker address when
> it tries
> &gt; &gt; lookup.
> &gt; &gt;
> &gt; &gt;
> &gt;
> https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1282-L1293
> &gt
> <https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1282-L1293&gt>;
> &gt;
> &gt; &gt; ## Goal
> &gt; &gt; Automatically release connections that are no longer used.
> &gt; &gt;
> &gt; &gt; - Scope
> &gt; &gt;&nbsp;&nbsp; - **Pulsar client**
> &gt; &gt; Contains connections used by consumers, Producers, and
> Transactions.
> &gt; &gt;
> &gt; &gt;&nbsp;&nbsp; - **Pulsar proxy**
> &gt; &gt; Contains only the connection between Proxy and broker
> &gt; &gt;
> &gt; &gt; ## Approach
> &gt; &gt; Periodically check for idle connections and close them.
> &gt; &gt;
> &gt; &gt; ## Changes
> &gt; &gt;
> &gt; &gt; ### API changes
> &gt; &gt; **ClientCnx** added an idle check method to mark idle time.
> &gt; &gt;
> &gt; &gt; ```java
> &gt; &gt; /** Create time. **/
> &gt; &gt; private final long createTime;
> &gt; &gt; /** The time when marks the connection is idle. **/
> &gt; &gt; private long IdleMarkTime;
> &gt; &gt; /** The time when the last valid data was transmitted. **/
> &gt; &gt; private long lastWorkTime;
> &gt; &gt; /** Stat. enumerated values: using, idle_marked, before_release,
> &gt; released**/
> &gt; &gt; private int stat;
> &gt; &gt; /**
> &gt; &gt;&nbsp;&nbsp; * Check client connection is now free. This method
> may change the state
> &gt; &gt; to idle.
> &gt; &gt;&nbsp;&nbsp; * This method will not change the state to idle.
> &gt; &gt;&nbsp;&nbsp; */
> &gt; &gt; public boolen doIdleCheck();
> &gt; &gt; /** Get stat **/
> &gt; &gt; public int getStat();
> &gt; &gt; /** Change stat **/
> &gt; &gt; public int setStat(int originalStat, int newStat);
> &gt; &gt; ```
> &gt; &gt;
> &gt; &gt; ### Configuration changes
> &gt; &gt; We can set the check frequency and release rule for idle
> connections at
> &gt; &gt; `ClientConfigurationData`.
> &gt; &gt;
> &gt; &gt; ```java
> &gt; &gt; @ApiModelProperty(
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name =
> "autoReleaseIdleConnectionsEnabled",
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "Do you
> want to automatically clean up unused
> &gt; connections"
> &gt; &gt; )
> &gt; &gt; private boolean autoReleaseIdleConnectionsEnabled = true;
> &gt; &gt;
> &gt; &gt; @ApiModelProperty(
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name =
> "connectionMaxIdleSeconds",
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "Release
> the connection if it is not used for more than
> &gt; &gt; [connectionMaxIdleSeconds] seconds"
> &gt; &gt; )
> &gt; &gt; private int connectionMaxIdleSeconds = 180;
> &gt; &gt;
> &gt; &gt; @ApiModelProperty(
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name =
> "connectionIdleDetectionIntervalSeconds",
> &gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "How
> often check idle connections"
> &gt; &gt; )
> &gt; &gt; private int connectionIdleDetectionIntervalSeconds = 60;
> &gt; &gt; ```
> &gt; &gt;
> &gt; &gt; ## Implementation
> &gt; &gt;
> &gt; &gt; - **Pulsar client**
> &gt; &gt; If no consumer, producer, or transaction uses the current
> connection,
> &gt; &gt; release it.
> &gt; &gt;
> &gt; &gt; - **Pulsar proxy**
> &gt; &gt; If the connection has not transmitted valid data for a long
> time, release
> &gt; &gt; it.
> &gt; &gt;
> &gt; &gt;
> &gt; &gt; Yubiao Feng
> &gt; &gt; Thanks
> &gt; &gt;
> &gt;

回复: [DISCUSS] [PIP-165] Auto release client useless connections

Posted by 一苇以航 <19...@qq.com.INVALID>.
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


------------------&nbsp;原始邮件&nbsp;------------------
发件人:                                                                                                                        "dev"                                                                                    <yubiao.feng@streamnative.io.INVALID&gt;;
发送时间:&nbsp;2022年6月6日(星期一) 凌晨1:08
收件人:&nbsp;"dev"<dev@pulsar.apache.org&gt;;

主题:&nbsp;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)&nbsp; 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&nbsp; and other commands.
The connection "For consumers, producers commands&nbsp; 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 <rgao@apache.org&gt; wrote:

&gt; This is a good idea, but I have a concern, Pulsar has the config
&gt; `brokerMaxConnections` to control max connection count against one broker.
&gt; If the connection is closed, it will re-connect when consumers or producers
&gt; start to consume and produce messages again, but this time the max
&gt; connection count will reach the max count.
&gt;
&gt;
&gt; On 2022/05/26 06:31:37 Yubiao Feng wrote:
&gt; &gt; I open a pip to discuss Auto release client useless connections, could
&gt; you
&gt; &gt; help me review
&gt; &gt;
&gt; &gt;
&gt; &gt; ## Motivation
&gt; &gt; Currently, the Pulsar client keeps the connection even if no producers or
&gt; &gt; consumers use this connection.
&gt; &gt; If a client produces messages to topic A and we have 3 brokers 1, 2, 3.
&gt; Due
&gt; &gt; to the bundle unloading(load manager)
&gt; &gt; topic ownership will change from A to B and finally to C. For now, the
&gt; &gt; client-side will keep 3 connections to all 3 brokers.
&gt; &gt; We can optimize this part to reduce the broker side connections, the
&gt; client
&gt; &gt; should close the unused connections.
&gt; &gt;
&gt; &gt; So a mechanism needs to be added to release unwanted connections.
&gt; &gt;
&gt; &gt; ### Why are there idle connections?
&gt; &gt;
&gt; &gt; 1.When configuration `maxConnectionsPerHosts ` is not set to 0, the
&gt; &gt; connection is not closed at all.
&gt; &gt; The design is to hold a fixed number of connections per Host, avoiding
&gt; &gt; frequent closing and creation.
&gt; &gt;
&gt; &gt;
&gt; https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionPool.java#L325-L335
&gt; &gt;
&gt; &gt; 2-1. When clients receive `command-close`, will reconnect immediately.
&gt; &gt; It's designed to make it possible to reconnect, rebalance, and unload.
&gt; &gt;
&gt; &gt;
&gt; https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConnectionHandler.java#L122-L141
&gt; &gt;
&gt; &gt; 2-2. The broker will close client connections before writing ownership
&gt; info
&gt; &gt; to the ZK. Then clients will get deprecated broker address when it tries
&gt; &gt; lookup.
&gt; &gt;
&gt; &gt;
&gt; https://github.com/apache/pulsar/blob/72349117c4fd9825adaaf16d3588a695e8a9dd27/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L1282-L1293
&gt; &gt;
&gt; &gt; ## Goal
&gt; &gt; Automatically release connections that are no longer used.
&gt; &gt;
&gt; &gt; - Scope
&gt; &gt;&nbsp;&nbsp; - **Pulsar client**
&gt; &gt; Contains connections used by consumers, Producers, and Transactions.
&gt; &gt;
&gt; &gt;&nbsp;&nbsp; - **Pulsar proxy**
&gt; &gt; Contains only the connection between Proxy and broker
&gt; &gt;
&gt; &gt; ## Approach
&gt; &gt; Periodically check for idle connections and close them.
&gt; &gt;
&gt; &gt; ## Changes
&gt; &gt;
&gt; &gt; ### API changes
&gt; &gt; **ClientCnx** added an idle check method to mark idle time.
&gt; &gt;
&gt; &gt; ```java
&gt; &gt; /** Create time. **/
&gt; &gt; private final long createTime;
&gt; &gt; /** The time when marks the connection is idle. **/
&gt; &gt; private long IdleMarkTime;
&gt; &gt; /** The time when the last valid data was transmitted. **/
&gt; &gt; private long lastWorkTime;
&gt; &gt; /** Stat. enumerated values: using, idle_marked, before_release,
&gt; released**/
&gt; &gt; private int stat;
&gt; &gt; /**
&gt; &gt;&nbsp;&nbsp; * Check client connection is now free. This method may change the state
&gt; &gt; to idle.
&gt; &gt;&nbsp;&nbsp; * This method will not change the state to idle.
&gt; &gt;&nbsp;&nbsp; */
&gt; &gt; public boolen doIdleCheck();
&gt; &gt; /** Get stat **/
&gt; &gt; public int getStat();
&gt; &gt; /** Change stat **/
&gt; &gt; public int setStat(int originalStat, int newStat);
&gt; &gt; ```
&gt; &gt;
&gt; &gt; ### Configuration changes
&gt; &gt; We can set the check frequency and release rule for idle connections at
&gt; &gt; `ClientConfigurationData`.
&gt; &gt;
&gt; &gt; ```java
&gt; &gt; @ApiModelProperty(
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name = "autoReleaseIdleConnectionsEnabled",
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "Do you want to automatically clean up unused
&gt; connections"
&gt; &gt; )
&gt; &gt; private boolean autoReleaseIdleConnectionsEnabled = true;
&gt; &gt;
&gt; &gt; @ApiModelProperty(
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name = "connectionMaxIdleSeconds",
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "Release the connection if it is not used for more than
&gt; &gt; [connectionMaxIdleSeconds] seconds"
&gt; &gt; )
&gt; &gt; private int connectionMaxIdleSeconds = 180;
&gt; &gt;
&gt; &gt; @ApiModelProperty(
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; name = "connectionIdleDetectionIntervalSeconds",
&gt; &gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; value = "How often check idle connections"
&gt; &gt; )
&gt; &gt; private int connectionIdleDetectionIntervalSeconds = 60;
&gt; &gt; ```
&gt; &gt;
&gt; &gt; ## Implementation
&gt; &gt;
&gt; &gt; - **Pulsar client**
&gt; &gt; If no consumer, producer, or transaction uses the current connection,
&gt; &gt; release it.
&gt; &gt;
&gt; &gt; - **Pulsar proxy**
&gt; &gt; If the connection has not transmitted valid data for a long time, release
&gt; &gt; it.
&gt; &gt;
&gt; &gt;
&gt; &gt; Yubiao Feng
&gt; &gt; Thanks
&gt; &gt;
&gt;

Re: [DISCUSS] [PIP-165] Auto release client useless connections

Posted by Yubiao Feng <yu...@streamnative.io.INVALID>.
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 <rg...@apache.org> 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
> >
> > 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
> >
> > 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
> >
> > ## 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
> >
>