michaeljmarshall opened a new pull request, #19377:
URL: https://github.com/apache/pulsar/pull/19377

   ### Motivation
   
   The current Elasticsearch sink has a setting named 
`connectionIdleTimeoutInMs`. This setting closes expired and idle http 
connections opened by the Elasticsearch Rest Client. The current default is 5 
milliseconds. With this value, I observed connection issues both for timeouts 
trying to connect to elasticsearch and errors like "connection reset by peer". 
When overriding the default to 5 seconds, all errors disappeared. I propose we 
change it to 30 seconds since even 5 seconds is a little low.
   
   ### Modifications
   
   * Change default value for `connectionIdleTimeoutInMs` from 5 millis to 
30000 millis
   * Remove unnecessary call to `closeExpiredConnections`. This call is 
unnecessary because the `closeIdleConnections` also closes expired connections. 
Source: 
https://hc.apache.org/httpcomponents-asyncclient-4.1.x/current/httpasyncclient/apidocs/org/apache/http/nio/conn/NHttpClientConnectionManager.html#closeIdleConnections(long,%20java.util.concurrent.TimeUnit)
   
   ```java
       /**
        * Closes idle connections in the pool.
        * <p>
        * Open connections in the pool that have not been used for the
        * timespan given by the argument will be closed.
        * Currently allocated connections are not subject to this method.
        * Times will be checked with milliseconds precision
        *
        * All expired connections will also be closed.
        *
        * @param idletime  the idle time of connections to be closed
        * @param tunit     the unit for the {@code idletime}
        *
        * @see #closeExpiredConnections()
        */
       void closeIdleConnections(long idletime, TimeUnit tunit);
   ```
   
   ### Verifying this change
   
   This is a trivial change. 
   
   ### Does this pull request potentially affect one of the following parts:
   
   - [x] The default values of configurations
   
   This PR changes a default value for a sink. The current default is 
problematic.
   
   ### Documentation
   
   - [x] `doc-not-needed`
   
   We generate the docs for this sink, so we don't need to update any site 
docs. We should include this update on the release notes.
   
   ### Matching PR in forked repository
   
   PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/22


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to