aymkhalil commented on code in PR #19377:
URL: https://github.com/apache/pulsar/pull/19377#discussion_r1092326905
##########
pulsar-io/elastic-search/src/main/java/org/apache/pulsar/io/elasticsearch/ElasticSearchConfig.java:
##########
@@ -214,10 +214,10 @@ public class ElasticSearchConfig implements Serializable {
@FieldDoc(
required = false,
- defaultValue = "5",
- help = "Idle connection timeout to prevent a read timeout."
+ defaultValue = "30000",
+ help = "Idle connection timeout to prevent a connection timeouts."
)
- private int connectionIdleTimeoutInMs = 5;
+ private int connectionIdleTimeoutInMs = 30000;
Review Comment:
I wonder if this config should be validated against
[bulkFlushIntervalInMs](https://github.com/apache/pulsar/blob/09504017526308fd83f480aa8cb502e1cfe0f633/pulsar-io/elastic-search/src/main/java/org/apache/pulsar/io/elasticsearch/ElasticSearchConfig.java#L189)
when bulk API is enabled - something like `connectionIdleTimeoutInMs > 2 *
bulkFlushIntervalInMs` because it seems the connection will set idle by design
in-between flushes
##########
pulsar-io/elastic-search/src/main/java/org/apache/pulsar/io/elasticsearch/client/RestClient.java:
##########
@@ -83,7 +83,6 @@ public RestClient(ElasticSearchConfig elasticSearchConfig,
BulkProcessor.Listene
// idle+expired connection evictor thread
this.executorService = Executors.newSingleThreadScheduledExecutor();
this.executorService.scheduleAtFixedRate(() -> {
- configCallback.connectionManager.closeExpiredConnections();
configCallback.connectionManager.closeIdleConnections(
Review Comment:
Q: Is it required at all to evict idle connections? I wonder what's wrong
with long lived connection that has a life cycle coupled with that of the sink
instance. If it is not required, we could drop the `connectionIdleTimeoutInMs`
for good but I maybe missing something.
--
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]
