<div dir="ltr"><div dir="ltr"><div></div><div><i>> Rob, why are you not using StructuredTaskScope instead of
Executors.newThreadPerTaskExecutor?</i></div><div><br></div><div>I'm not in the Helidon team nor do I work for Oracle so I'm not the correct person to ask. I have seen commented out <i>StructuredTaskScope </i>code in Nima though so there is possibly a commit comment to find. I suspect it's just the case that Nima is building against the released JDK that does not include <i>StructuredTaskScope</i> yet?<br></div><div><br></div><div><br></div><div><div><i>> Basically, is there something you know that I don't know?</i></div><div><br></div><div>Unlikely :) To me the use of <i>Executors.newThreadPerTaskExecutor </i>does look appropriate (with just this issue around graceful shutdown which is important to me due to how it impacts Nima running in Kubernetes)<i>.<br></i></div><div><br></div></div><div><br></div><div><i>> who has been playing with
Nima</i></div><p>I don't really want to side track this topic too much. I'll add my Nima comments afterwards. <br></p><p><br></p><p>Cheers, Rob.<br></p></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 21 Dec 2022 at 10:10, Eric Kolotyluk <<a href="mailto:eric@kolotyluk.net" target="_blank">eric@kolotyluk.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>
<div>
<p>As another curious casual observer, who has been playing with
Nima...</p>
<p>Rob, why are you not using StructuredTaskScope instead of
Executors.newThreadPerTaskExecutor?</p>
<p>Basically, is there something you know that I don't know?</p>
<p>Cheers, Eric<br>
</p>
<div>On 2022-12-20 1:01 p.m., Rob Bygrave
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div><i>> The way to interrupt sockets has always been to
close them</i></div>
<div><br>
</div>
<div>We are looking to achieve "Graceful shutdown" which means
it allows in-flight requests to complete before shutting
down. We don't want to just nuke em or said differently, it
is the job of executorService.awaitTermination(...)
executorService.shutdownNow() to nuke anything still active
after the timeout.</div>
<div><br>
</div>
<div>... yes we want to interrupt Virtual Threads that are
WAITING and known to be <i><b>not in-flight</b></i> <i><b>BEFORE</b></i>
executorService.awaitTermination(...)</div>
<div><br>
</div>
<div><br>
</div>
<div><i>> anything to do with Loom and virtual threads.</i></div>
<div><br>
</div>
<div>Maybe. There are a few things that make me ask though and
I'm in particular looking at ThreadPerTaskExecutor. One is
that all the existing Java http servers that I know of use NIO
(as you would expect) and specifically that means "getting the
next request" for http 1.1 keepalive true is an <i><b>event</b></i>
where as with Virtual Threads and Nima "getting the next
request" is a Virtual Thread blocking on IO waiting for those
next bytes to arrive (which to me is the "new normal" approach
that Virtual Threads brings).</div>
<div><br>
</div>
<div>That is, with Virtual Threads I am guessing that it's
potentially a "new normal" pattern where we are looking to
shutdown gracefully a ThreadPerTaskExecutor which is holding
active Virtual Threads that are blocking waiting on IO (but
not yet deemed in-flight). I'm suggesting that
ThreadPerTaskExecutor currently does not make it nice / easy /
friendly to shutdown gracefully allowing for in-flight
requests. <br>
</div>
<div><br>
</div>
<div>I see ThreadPerTaskExecutor.shutdownNow() always returns an
empty list of Runnable (this is too late in the shutdown
process anyway so not useful per say but we are trying to
access the Stream<Runnable>). I see the internal
ThreadContainer and the internal Stream<Thread>
threads() method but I don't see anything available to
application code to assist the shutdown of a
ThreadPerTaskExecutor for this case which I feel might become
a common pattern going forward. </div>
<div><br>
</div>
<br>
<br>
<div>Cheers, Rob.<br>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, 21 Dec 2022 at 05:38,
Jens Lideström <<a href="mailto:jens.lidestrom@fripost.org" target="_blank">jens.lidestrom@fripost.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<br>
Random passer-by here.<br>
<br>
I don't think this has anything to do with Loom and virtual
threads.<br>
<br>
The way to interrupt sockets has always been to close them.
Then blocking methods will throw an exception.<br>
<br>
See this, for example: <a href="https://stackoverflow.com/questions/4425350/how-to-terminate-a-thread-blocking-on-socket-io-operation-instantly" rel="noreferrer" target="_blank">https://stackoverflow.com/questions/4425350/how-to-terminate-a-thread-blocking-on-socket-io-operation-instantly</a><br>
<br>
Nima seems to do that for the server socket, but it FIRST
waits for the ExecutorService to shutdown:<br>
<br>
<a href="https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ServerListener.java#L137" rel="noreferrer" target="_blank">https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/ServerListener.java#L137</a><br>
<br>
I think Nima should close the socket FIRST, then shutdown the
ExecutorService.<br>
<br>
(I have not checked how Nima handles the client sockets. They
should be treated in the same way.)<br>
<br>
Having written the above it strikes me that maybe you aware of
this already? Maybe you are suggesting the addition of some
new kind of application independent interruption mechanism
that are honoured by InputStreams from Sockets? I don't think
this is related to Loop, either.<br>
<br>
Best regards,<br>
Jens Lideström<br>
<br>
<br>
On 2022-12-19 22:51, Rob Bygrave wrote:<br>
> I have been looking at the shutdown process of Helidon
Nima web server which makes use of:<br>
> <br>
> Executors.newThreadPerTaskExecutor(Thread.ofVirtual()<br>
> .allowSetThreadLocals(true)<br>
> .inheritInheritableThreadLocals(false)<br>
> .factory());<br>
> <br>
> Firstly, there is no issue with Nima web server shutdown
when using HTTP 1.0 no keepalive. The virtual threads for this
case live for the length of a single request/response only.<br>
> <br>
> Where I am hitting a question/issue is with Nima web
server shutdown when using HTTP 1.1 keepalive true (and there
is at least 1 connection being kept alive). What happens with
HTTP 1.1 with keepalive true is that there is 1 virtual thread
per connection (slight simplification). After the
request/response has been processed the virtual thread then
looks to read the next request. When there are no more
requests coming into the web server we see this thread is
WAITING looking to read the first part of the next request.<br>
> <br>
> The thread is WAITING "while reading the prologue" here:<br>
> <a href="https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java#L115" rel="noreferrer" target="_blank">https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java#L115</a>
<<a href="https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java#L115" rel="noreferrer" target="_blank">https://github.com/helidon-io/helidon/blob/main/nima/webserver/webserver/src/main/java/io/helidon/nima/webserver/http1/Http1Connection.java#L115</a>><br>
> <br>
> Conceptually when the Nima web server is "idle" we expect
to be able to shutdown the web server gracefully (allowing for
active requests to complete) and quickly. In this "idle" state
with HTTP 1.1 keepalive connections the ThreadPerTaskExecutor
contains alive threads that are in WAITING state while
"reading the prologue of the next request".<br>
> <br>
> Currently the webserver.stop() ends up as the usual:<br>
> <br>
> executorService.shutdown();<br>
> if (!executorService.awaitTermination(...)) {<br>
> executorService.shutdownNow()<br>
> }<br>
> <br>
> Where the executorService in question is
ThreadPerTaskExecutor. The above shutdown does not execute in
a timely manner based on the timeout used for
executorService.awaitTermination(...) - for example if this
timeout is 10 seconds we get a pretty slow shutdown on a
conceptually idle web server.<br>
> <br>
> <br>
> *Some thoughts*<br>
> An approach would be as part of
[ThreadPerTaskExecutor].shutdown() to firstly look to
interrupt threads that are state == WAITING &&
some-application-specific-logic-that-says-the-task-is-interruptible
(which for Nima is that the task is readPrologue() at
Http1Connection.java#L115).<br>
> <br>
> e.g. Perhaps have a interface InterruptableTask extends
Runnable ... and on shutdown() firstly iterate the active
threads that are WAITING and if their tasks are
InterruptableTask and
application-specific-logic-that-says-the-task-is-interruptible
true then interrupt that thread.<br>
> <br>
> I note there is some non-public API like
Stream<Thread> threads() which applications can't use
(at least yet). My current hacking around with this shutdown
in Helidon hasn't given me any elegant solutions in
application logic in that we need to now have the application
additionally hold the Runnable tasks (which is not ideal in
the http 1.0 no keepalive case).<br>
> <br>
> <br>
> Does anyone have some thoughts or suggestions on this?<br>
> <br>
> <br>
> I have raised this as an issue in Helidon as: <a href="https://github.com/helidon-io/helidon/issues/5717" rel="noreferrer" target="_blank">https://github.com/helidon-io/helidon/issues/5717</a>
<<a href="https://github.com/helidon-io/helidon/issues/5717" rel="noreferrer" target="_blank">https://github.com/helidon-io/helidon/issues/5717</a>><br>
> with a related PR with failing tests. I'm raising this
here because my current thought is that an elegant solution
might be with ThreadPerTaskExecutor on shutdown or some
exposure of the waiting threads accessible to application
code.<br>
> <br>
> <br>
> Thanks, Rob.<br>
> <br>
</blockquote>
</div>
</blockquote>
</div>
</blockquote></div></div>