Micronaut-core: Add a configuration option to disable automatic thread pool selection

[martinandersson]

TL;DR

The user guide is fragmented and inconsistent/unclear as to what thread will
invoke my @Controller method and/or subscribe to a controller-returned reactive
type (Publisher/CompletableFuture/Whatever).

Will it 1) be a thread from the Netty event loop group (which we don't want to
block) or will it 2) be a thread from an unbounded I/O thread pool (which we may
block while he awaits a result from I/O)?

My impression of the user guide and what I think was the intended and desired
baseline behavior for Micronaut is to execute a controller method using the
Netty event loop thread for all return types except reactive ones in which case
the method is executed and/or the subscription is performed using an I/O thread.

But this is within the limits of my guesswork. Some statements from the
user guide run counter to the aforementioned conclusion and if true, actually
suggest that the Netty event loop thread will be blocked while waiting on I/O
(!).

Furthermore, whatever assumption Micronaut makes about my code (return type),
what is the consequence? Thread X will execute my method or thread X will
subscribe to the result? Because this is two completely different things!

Please note that Micronaut's automagic "thread branching" seems to be fully
based on the return type of the controller. It is unclear - but also outside the
scope of my concern to be honest - what effect reactive types have if declared
as method parameters.

What the userguide has to say

I will group each of these quotes according to:

• Return type; reactive or non-reactive.
• Method executor; Netty or I/O.
• _Only applicable for reactive return types;_ The subscriber. Netty or I/O.

This grouping will help summarize the user guide's many statements later on.

6.11 Reactive HTTP Request Processing:

If your controller method returns a non-blocking type such as an RxJava
Observable or a CompletableFuture then Micronaut will use the Event loop
thread to subscribe to the result.

If however you return any other type then Micronaut will execute your
@Controller method in a preconfigured I/O thread pool.

This quote says that for reactive types, it is unknown what thread calls my
method but the Netty thread will be used to subscribe to the result. For
non-reactive types, I/O will execute the method [and there is no result to
subscribe to].

Which is put like this (for future reference):

Reactive > ? > Netty
Non-reactive > I/O

As will become evident later in this report, it seems like Micronaut strictly
thinks of reactive return types as a description of I/O work versus non-reactive
return types which are "CPU only" because their result is imminent and not
pushed back into the future.

But, the quote above runs in direct opposition to this and suggests two bad
things; the Netty thread will subscribe to and await I/O result and the I/O
thread will - for no added advantage - be the one used to fetch a directly
available result.

6.11.2 Reactive Responses:

If the request is considered non-blocking (because it returns a non-blocking
type) then the Netty event loop thread will be used to execute the method.

If the method is considered blocking then the method is executed on the I/O
thread pool, which Micronaut creates at startup.

Reactive > Netty > ?
Non-reactive > I/O

This quote is almost the same as the last one. The only difference here is that
the Netty thread is said to _execute_ the method instead of _subscribing_ to the
result (nonetheless a very important detail that ought to be cleared!).

6.12 JSON Binding with Jackson

_(The user guide has an example of a controller method returning a reactive
Single<Person>, then says:)_

Note however, that if your method does not do any blocking I/O then you can
just as easily write:

_(..and changes the example to return an unwrapped Person. Then writes:)

In other words, as a rule reactive types should be used when you plan to do
further downstream I/O operations in which case they can greatly simplify
composing operations.

Nothing is said about which thread do what, but I must deduce:

Reactive > I/O > ?
Non-reactive > Netty

This quote breaks away from the previous pattern and instead flip the assumption
made from reactive return types. Now - and forever after in the user guide - a
reactive return type is equated with "I/O operations" and the inverse for
non-reactive types.

6.18 Writing Response Data:

Micronaut's HTTP server supports writing data without blocking simply by
returning a Publisher the emits objects that can be encoded to the HTTP
response.
...
The server will request a single item from the Publisher, write the item,
without blocking, and then request the next item, thus controlling back
pressure.
...
When returning a Writable object the blocking I/O operation will be shifted to
the I/O thread pool so that the Netty event loop is not blocked.

Reactive > ? > I/O
Non-reactive > Netty

This quote is very fuzzy. The "server" is either "blocking" or "not blocking"
whatever that means, but it sure does smell like a continuation of the last
quote. I.e., a stone-cold assumption is made that a reactive return type is
equal to an "I/O operation".

6.21 HTTP Filters

What you don't want to do is block the underlying Netty event loop within your
filter, instead you want to the filter to proceed with execution once any I/O
is complete.
...
The RxJava I/O scheduler is used to execute the logic

The quote is text that decorated an example of a request filter. Two interesting
notes can be made.

1. Request filters do not get the same thread branching treatment as controllers
   do and must make the decision where to run the code explicitly.
2. But the same ideology as before is still pushed through: a reactive return
   type must be equal to I/O activity!

The summary

The user guide is massively confused what happens with reactive return types with
all references essentially saying different things:

Reactive > ? > Netty
Reactive > Netty > ?
Reactive > I/O > ?
Reactive > ? > I/O

Similarily, there's two opposite camps on what happens to non-reactive types:

Non-reactive > I/O
Non-reactive > Netty

Please note that it is that small detail "executes my method" versus "subscribes
to the result" which further causes a rift in the reactive group.

However; most text, most references and all code examples provided in the
user guide are in agreement of the following:

A _controller method that has a "normal" Java return type is executed by the
Netty event loop thread. A reactive return type represents I/O work and the
method will be executed and/or the result will be scheduled using an I/O
thread._

Let's see if that is true!

My empirical observation

This is a super simple Endpoint/Controller that echoes back identification of the
calling thread:

import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.reactivex.Single;

import java.util.Map;

@Controller("/thread")
public class ThreadEndpoint
{
    static final class ThreadInfo {
        public final String id, name, group;

        ThreadInfo() {
            Thread t = Thread.currentThread();

            id = String.valueOf(t.getId());
            name = t.getName();
            group = t.getThreadGroup().getName();
        }
    }

    @Get("/nonreactive")
    Map.Entry<String, ThreadInfo> nonReactiveReturnType() {
        return entryOfThread("executor");
    }

    @Get("/reactive")
    Single<Map<String, ThreadInfo>> reactiveReturnType() {
        var executor = entryOfThread("executor");

        return Single.fromCallable(() ->
                Map.ofEntries(executor, entryOfThread("subscriber")));
    }

    private static Map.Entry<String, ThreadInfo> entryOfThread(String key) {
        return Map.entry(key, new ThreadInfo());
    }
}

Yields on my machine:

foo@bar:~$ curl localhost:8080/thread/nonreactive
{
  "executor": {
    "id": "27",
    "name": "pool-1-thread-6",
    "group": "main"
  }
}

foo@bar:~$ curl localhost:8080/thread/reactive
{
  "executor": {
    "id": "35",
    "name": "nioEventLoopGroup-1-8",
    "group": "main"
  },
  "subscriber": {
    "id": "35",
    "name": "nioEventLoopGroup-1-8",
    "group": "main"
  }
}

Okay, so the Netty event loop thread is used to execute and subscribe to methods
with a reactive return type. Or to put it differently; most applications who can
be assumed to follow your code examples and use reactive types for I/O will
block the event loop (!).

Further, any "normal application code" who return normal types will incur the
overhead cost of submitting CPU-bound work to a thread pool other than that
already dealing with the inbound request.

And just for the record, the event loop thread is already pooled (see this
and that):

If not specified else, the Executor used by default is a ForkJoinPool.

The default value is the value of the system property
io.netty.eventLoopThreads or if not specified the available processors x 2

Suggestions of improvement

Don't add semantics

It's unclear what a reactive type is to Micronaut. The overall idea and
certainly what all code examples illustrate is a direct association between "I/O
work" and reactive types.

But a reactive type is whatever we make out of it. It can be blocking, or not
blocking, or whatever. Micronaut's own user guide literally changes his mind on
this!

Furthermore, reactive programming is absolutely not all about asynchronicity and
making multi-threaded programs easier to write. The reactive API offers a whole
lot more; streaming (filtering, mapping, combining), backpressure (potentially
unbounded data sets), operators (reduce, replay), error handling and so forth.

In fact, the whole API is single-threaded "by default" (see this and
that):

By default, an Observable and the chain of operators that you apply to it will
do its work, and will notify its observers, on the same thread on which its
Subscribe method is called.

Adding Micronaut-specific semantics effectively forces the developer to be very
cautious about how he uses reactive types and for what purpose. Or to put it
differently; it adds a new specification on top of an already well-established
specification. I can not see the benefit in this.

What I have learned though is that most Micronaut programs out there who
followed the user guide probably block the Netty event loop thread. Even though
the original intent I am sure of was all good, the "icing on the cake" has
incurred a whole lot of time from developers trying to understand the magic and
"rules" which the framework applies.

Complexity should be our worst enemy

When does what "rule" kick in? Is it when my method returns a "reactive type"?
Maybe when it returns a CompletableFuture, a Writable? Something else? Will
it be executed by what thread? Who subscribes to the result? Is this only for
methods declared inside a Controller? Inside a Filter? Something else? What
happens if..

Reducing complexity makes the code easier to reason about. And this rule I would
argue should be the pinnacle of any type of framework. A framework is supposed
to make my life easier, not harder.

There should only be one way

As my empirical observation showed, say returning a non-reactive String will
make Micronaut run the method using the I/O thread pool. If I wanted to escape
the Netty event loop thread, I would very simply use the @Async annotation,
the reactive operator subscribeOn() or any other standardized method. Typing
in one little simple word like this in the source code is far more readable,
less error-prone and less time-consuming than having to "fight" complexity added
by Micronaut.

The current state of affairs is dire, to be honest. But there is one simple and
elegant solution to all of this: Remove the notion of automagic
thread-branching. Let developers be developers =)

[graemerocher]

So interesting feedback, thank you. Generally in terms of why this decision was made and why it is like this, is that we cannot guarantee that a method that returns a non-reactive type (say String) doesn't block. So we shift that work onto another thread pool. There is an escape hatch which is to add the @NonBlocking annotation to the method or class. In which cases it will run the logic on the event loop.

If you return a reactive type then yes we run the logic on the event loop. The assumption here is that typically you engage a non-blocking library that returns reactive types like Mongo Reactive driver or whatever. There is equally the ability to add @Blocking to the method that returns a reactive type so it scheduled on the I/O thread pool even if it returns a reactive type.

If you are doing something like the following:

return Single.fromCallable(() ->
    // some blocking call here
);

Then I fail to see the value of reactive here. You are just adding complexity to your code for no reason whatsoever. If you are going to do this, then you should add a call to subscribeOn(..) and pass the appropriate scheduler.

I don't see why the situation is particular "dire". The behaviour is detailed in the user guide, however if there are ambiguities then we absolutely encourage you to submit PRs to the documentation to add clarifications.

Other than that I am not sure where to take this issue from here, is this an improvement request? Would you like an option to always run it on the event loop? To disable this behaviour?

If you are looking for more insight into how non-blocking JSON is processed we could certainly provide that. The guts of it are in JacksonProcessor which uses the Jackson async processing to read data chunk by chunk https://github.com/micronaut-projects/micronaut-core/blob/master/runtime/src/main/java/io/micronaut/jackson/parser/JacksonProcessor.java

I am not sure it adds a great deal of value for every user to know deep levels of technical detail how that works in that particular section of documentation, but maybe I am wrong.

Nevertheless how do we take this issue forward?

[martinandersson]

Thank you for the added subject matter!

The user guide is more than ambiguous. It contradicts itself. First, it associates only non-reactive types with blocking I/O but later flips this around and for ALL code examples provided, it uses reactive types to do blocking I/O.

I.e., all code examples and anyone following this trail will cause the Netty event loop to be blocked. So yes, I think that this is pretty "dire". I mean no offense by this word, but I do suspect that most Micronaut applications out there doing "standard I/O" will wrap this work in a reactive type and thus block the event loop which is really really bad.

The "confusion" and contradicting statements in the user guide - i.e. entries made by Micronaut experts - highlights the point I am trying to make here: You can make out of a reactive type or a non-reactive type whatever you want to. The only thing true is that I as the application developer is the only one who knows what is going on inside my method. And if I do anything "nonstandard" then I better also use any one of all the tools already provided to me.

I am glad that it turns out you run reactive types using the Netty thread. This is the only "standard behavior" I would have expected. But Micronaut assumes all non-reactive types are I/O? This is a very bold assumption to make and I would bet it's not very typical. Given what we already assume about reactive types, then the inverse must be true that non-reactive types represent an immediate result without blocking.

And down the rabbit hole we go with even more added annotations; the escape hatch @Blocking and @NonBlocking. Complexity gives birth to complexity. Wouldn't it be far simpler to remove this magic behavior and not make any assumptions at all? Especially given that the @Async annotation already exists in Micronaut's own API and subscribeOn() exists in the reactive API.

Not that I mind the small performance loss (if even noticeable) running my "normal" methods in an I/O thread pool, but this just isn't the right semantics applied. The I/O thread pool is for I/O work. Moving forward, I will have to add @NonBlocking to all my methods. This will make my code look weird lol.

Regarding the docs. I consider them to be a "specification". I know now after my research that "non-reactive return types" will be executed in the I/O thread pool. Sure, I could add @NonBlocking to most of my methods and the ones that do I/O I can just boldly go ahead and do my blocking I/O work without minding about the event/request thread (oh, just thinking about it causes me a shivering feeling). But as long as the docs are contradicting, then the framework's behavior can also change at any notice so I essentially have no model to lean against.

I would love to contribute to the docs. Micronaut is one of the coolest things I have the honor to learn. But the problem here is that I honestly can not stand behind the idea of having a framework make assumptions about what's inside my methods and basically treat me like a baby lol. To me, it is alien to have a framework not provide me with 1 rock hard solid programming model. I.e., have all requests be handled by the Netty event loop pool, or hey.. let Micronaut run it's own request-dealing thread pool if blocking is that much of a concern (this would really be "icing on the cake"). But the idea of having a web framework run my code somewhere else in just some cases based on a return type is.. I just can't cope with it. My brain says no! hahahaha

Also, just for the record and my learning curve here.. is it true that filters do not have this thread-branching behavior applied as the user guide example implied? Filters are explicit; "what you do is what you get" kind of a thing?

hahahaha see, I wouldn't have had to ask that question if we stopped doing magic.

[martinandersson]

So sorry for my babbling. If it isn't clear already I just want to give a direct answer to your last question: I think we should remove the thread-branching behavior (and hacks such as @Blocking/@NonBlocking). No magic! Let users use the already provided methods such as @Async/subscribeOn() or whatever else the standard Java library provides.

This would also render the fix for the user guide super simple because we could just remove all the text trying to explain Micronaut's current magic. To be really friendly though, we could provide an information something like this:

"Hey, as you know, Micronaut runs on top of Netty. Netty's event loop thread will execute all application code. This is fine for work that is CPU-intensive but any I/O work - i.e. waiting on external hardware - should be threaded off appropriately (see @Async et cetera) or else your application might stop responding to new requests while at the same time idling doing nothing."

[graemerocher]

Appreciate the feedback. We will take this under consideration for Micronaut 2.0. We cannot just simply change the behaviour in 1.x as it would be a breaking change and we are trying to adhere by semantic versioning.

What we can do however it make the behaviour configruable through a setting that you can specific in your configuration to either enable or disable the automatic thread switching. Does this sound like a reasonable suggestion?

In terms of your other questions, Filters do not have any thread branching behaviour applied no. I believe this is consistent since filters return a publisher which is a reactive type, correct me if I am wrong.

One final thing to note is that @Blocking and @NonBlocking can be applied at the type level or even on an interface that you classes implement if you want to make it global and not have to repeat it for each method.

[martinandersson]

A configurable setting sounds cool =)

[martinandersson]

I just had the chance to see real-world Micronaut projects within an international organization who are active in global medicine and healthcare. My specific task was to improve their throughput and performance which they were not happy with.

Just as I feared, they had declared reactive types everywhere and was wrapping all of their I/O work in a Single.fromCallable(). This means that despite their best intent and effort, they were actually blocking the Netty event loop group thread in all microservices across the entire domain.

We talked about this and the developers simply shrug their shoulders and told me that all they had been doing was to "follow the user guide".

This is exactly what I feared and what I also described in this thread. Of course I have not conducted scientific research on this, but I am pretty sure at this point that we can expect most of all Micronaut applications out there in production across the entire world to be blocking the Netty thread and possibly perform much worse- and run with a performance completely counter to what was originally planned and designed for.

I can not stress enough how dire I think it is to have the user guide wrongfully describe a bad pattern as "best practice", let alone have Micronaut perform this automatic thread-switching in the first place. A new major release should be stressed that completely removes this behavior and all the patch-work such as @Blocking and @NonBlocking. Further, with the smallest amount of delay the user guide must be updated. The impact of taking no action could potentially be staggering and limit the adoption of Micronaut.

I feel like adding configuration on top of the framework - however temporarily and transient in nature - is only going to make the framework more complex and hard to understand. I see little value in this. My vote is to immediately update the user guide, not add any configuration options, and as quickly as possible remove the automatic thread-switching. This topic is far too important and has an enormous weight, I don't believe we can afford to take it lightly.

[graemerocher]

Thanks for the feedback. Changing this behaviour does indeed require a new major release since we following semantic versioning. Unfortunately that cannot happen "immediately" as roadmap planning doesn't allow for that nor can we release a new major version that does not provide an upgrade path for users who already take advantage of this behaviour.

So for now I have added the configuration option to 1.x which is currently set to behave how Micronaut currently behaves. You can set:

micronaut:
    server:
        thread-selection: MANUAL

To enable manual thread selection, which we will consider making the default for Micronaut 2.0. However like I said we cannot simply switch to this new behaviour without supplying a path forward for users who are going to be upgrading from Micronaut 1.x hence a configuration option is needed regardless whether that introduces additional complexity or not.

[dfernandezm]

Hi, thanks both for such detailed discussion about thread behaviours in Micronaut. I am coming here from 3 years using Vert.x (and 10 using Spring) and got very confused with the behaviour of Micronaut, how and when switches threads and the purpose, I would expect a very clear and predictable behaviour.

Vert.x for example is crystal clear in the docs about the dangers of blocking the event loop and has a built in thread monitor to warn when it’s being blocked. This is useful, and prescriptive alongside its explicit ‘executeBlocking’ operation.

I am pushing a large greenfield strategic project in a somewhat outdated company to use Micronaut, as my investigations give the best of all the worlds (including my bias for Vert.x). But I will have to onboard a team of developers into the reactive non-blocking world and if it feels even more confusing from the beginning it’s going to become difficult to justify.

I appreciate your consideration to add this in a major release and have the configuration option but, in my opinion, the user guide should be crystal clear stating the sound practices about using Netty event loops, avoid blocking them and how not doing it will slow down applications.

Thanks for your awesome work, looking forward to start my journey as Micronaut adopter.

[graemerocher]

@martinanderssondotcom @dfernandezm PR https://github.com/micronaut-projects/micronaut-core/pull/2830 suggests a change to the default behaviour for Micronaut 2.0 and documentation clarifications. I am interested to hear your feedback as to whether this change eliminates confusion caused by the current implementation and documentation.

[martinandersson]

Graeme, big thank you for this change =) I welcome very much removing the automatic thread selection.

However, I am personally in a one-man war against all annotations and magic. So I can not really "condone" the type @ScheduleOn.

The first thing I immediately ask is; exactly which types does this annotation apply to? Surely not Future as these could already be running. What would Micronaut do then if I specify another scheduler/thread pool? Cancel the task?

What happens if I annotate my method with @Blocking/@NonBlocking _and_ @ScheduleOn? Perhaps I'll sprinkle the method with an @Async, too. Can you imagine how many questions on Stackoverflow this will spawn?

It requires someone borderline obsessive and experienced to understand how subscribeOn() affects the upstream and observeOn() affects the downstream. Micronaut adding @ScheduleOn is not going to help 99% of all developers out there trying to figure things out.

But, here is where I would argue the painting really starts to crack:

Why would we add this annotation, what good will come out of it?

This annotation will (or rather, "should") do nothing but cause Micronaut to invoke a certain function for a specific subset of all "reactive types" out there that has this function declared to begin with (the fully equivalent function on types known ahead of time must be declared and used, otherwise the semantics can not be guaranteed).

If and when a developer realizes after fighting for days figuring out magic that all he had to do was to type in ".subscribeOn()" instead of "@ScheduleOn", then I have a feeling he will be left feeling Micronaut simply isn't considerate and trying to be "out of his way". Which, is the one and only noble goal any framework should strive for, if you ask me.

Look, where do we stop? Last time I checked RxJava's Observable literaly had 470 protected+public methods declared (!). Is the idea to add a new annotation for each one?

The way I see this, the only effect this new annotation has is to cause confusion, lots of potential bugs, a greater work load on documentation/learning curve, and last but not least, completely void of any benefits.

In all other use cases this wouldn't have been okay. We don't write code for nothing. In fact, we love deleting code and to simplify things. And we are - or at least should be in my opinion - against "decorators" with no added value, type explosion, complexity, ...

Having an annotation which is a hack to have my framework call a certain method on my return type instead of me just calling the method is ..... OMG! Words can not describe what I feel about this. As framework authors we have a responsibility to guide our users. Believe you and me, we could throw in a hundred completely meaningsless annotations in there and they would all find their way into blogs, books and real-world projects I'm sure of. Soon enough, Micronaut has turned into the new Spring.

Erm.. I think I have voiced my opinion enough for now hahahaha. I am so sorry. I have lots of opinions lol. To summarize - taking a deep breath - I think that we should remove @ScheduleOn, @Blocking and @NonBlocking. Probably even @Async but this is another discussion to be had.

Again, super good that we're moving forward on this. I am really happy. I will sleep better now hahahaha

[graemerocher]

@martinanderssondotcom Whilst I understand your concern if you are against annotations then you are maybe using the wrong framework I am afraid.

If we omit to include @ScheduleOn the net result is that every interaction with a blocking API such as JDBC users will have to wrap their logic in code like this forcing users into RxJava land:

    @Get("/{name}")
    Single<Person> byName(String name) {
        return Single.fromCallable(() ->
                personService.findByName(name) 
        ).subscribeOn(scheduler); 
    }

That IMO is an undesirable end result and exactly what annotations were designed for ie. to specify cross cutting concerns (@Transactional , @Cacheable etc.).

It will also be a much greater upgrade challenge for users who are migrating to Micronaut 2.0 if the upgrade instructions are "wrap all your code in this RxJava block" as oppose to "add this annotation to your controller". So we have multiple things to consider.

It may be hard for you to believe but we also have users who do not interact with RxJava at all when using Micronaut. For these users whether it is scheduleOn or what operator it maps to is irrelevant, and but an implementation detail.

RxJava is in fact an optional part of the public API and not something we necessarily force onto users.

As for @Blocking and @NonBlocking. Even if those annotations do not impact the functionality of the application after this change they are in fact useful from a documentation point of view.

Finally, semantically a Future would behave the same as a Single in that the framework will await the result on the thread pool specified by @ScheduleOn

[martinandersson]

So you're saying that @ScheduleOn does more than just a call to .subscribeOn()?

Because to me that wasn't apparent. Currently, all the JavaDoc of this annotation is basically this: "used to indicate which executor service a particular task should run on". I took this to simply mean a decorative call to .subscribeOn().

I mean the text isn't wrong for reactive return types (it is wrong for Future and CompletionStage), but maybe we ought to dwell into details there and explain exactly what is going on?

Details such as exception translation matters now too. What kind of error handler will the wrapper Maybe/Single use? What will happen with the Single's "native" NoSuchElementException?

I was always used to be having only one error handling model in whatever framework I used. If we do add this reactive magic on top of everything then I am thinking maybe we want to "unwrap" known and expected exceptions from the reactive provider (OnErrorNotImplementedException, + maybe something else?) and drop these causes into the normal exception handling workflow the user is acquainted with.

Or we can just drop the annotation and make the world a bit more explicit and simple! =)

[dfernandezm]

@graemerocher First of all, thanks a lot for considering the feedback.

I do really think the changes you’re making lean towards the idea of ‘don’t ever block the event loop, but use it as much as you can’. I will admit that my judgment is a bit biased towards Vert.x behaviour and its mantra of explicitly moving blocking workloads outside the event loop, no magic involved. But I do understand that the impact of the change is well supported with the annotations and explicit explanations.

In my opinion, as long as RxJava can still be surfaced at any point and do manual subscribes, the choice is from the dev: use the tools Micronaut gives or handle it yourself.

Keep up the awesome work on Micronaut.

[morki]

@graemerocher what about coroutines here?

Will it setup CoroutineContext with selected dispatcher? Now there is EmptyCoroutineContext