andreachild commented on code in PR #2915:
URL: https://github.com/apache/tinkerpop/pull/2915#discussion_r1861117495
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java:
##########
@@ -146,6 +146,11 @@ public synchronized
CompletableFuture<ServerGremlinExecutor> start() throws Exce
b.childOption(ChannelOption.WRITE_BUFFER_WATER_MARK,
new WriteBufferWaterMark(settings.writeBufferLowWaterMark,
settings.writeBufferHighWaterMark));
b.childOption(ChannelOption.ALLOCATOR,
PooledByteBufAllocator.DEFAULT);
+ // Enable TCP Keep-Alive to detect if the remote peer is still
reachable.
+ // Keep-Alive sends periodic probes to check if the remote peer is
still active.
+ // If the remote peer is unreachable, the connection will be
closed, preventing resource leaks and
+ // avoiding the maintenance of stale connections.
+ b.childOption(ChannelOption.SO_KEEPALIVE, true);
Review Comment:
Hello @dh-cloud, thanks for noticing this issue with potential resource
leaks in the gremlin-server.
In the master branch we have switched to HTTP (away from web sockets in 3.7)
which means there will be much more short term connections compared to web
sockets which uses long lived connections. When there is decreased activity we
should aim to release the resources and thus not use the keep alive option to
keep the connection active.
There is still an issue with potential resource leak but instead we can fix
this using the existing `idleConnectionTimeout` setting. The issue here is that
during the switch from web sockets to HTTP, we neglected to enable idle
connection monitoring for HTTP 😢 . What do you think about changing this PR
to enable idle connection monitoring instead?
See below for references in the code as to what I'm referring to:
*
https://github.com/apache/tinkerpop/blob/56c72f600c6e64a32bb5acf6115b29bdced51d17/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java#L167
*
https://github.com/apache/tinkerpop/blob/56c72f600c6e64a32bb5acf6115b29bdced51d17/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Channelizer.java#L52
*
https://github.com/apache/tinkerpop/blob/56c72f600c6e64a32bb5acf6115b29bdced51d17/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java#L144
--
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]
