Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@Body InputStream Blocks nioEventloop when its not read #6100

Closed
sumitaneja opened this issue Sep 6, 2021 · 18 comments · Fixed by #6120
Closed

@Body InputStream Blocks nioEventloop when its not read #6100

sumitaneja opened this issue Sep 6, 2021 · 18 comments · Fixed by #6120
Assignees
Labels
status: example attached status: pr submitted A pull request has been submitted for the issue type: bug Something isn't working

Comments

@sumitaneja
Copy link

Expected Behavior

When using large payloads say approx 5 mb, Expected to be body read to happen on IO threadpool. Tried Both micronaut 2.5.5 and 2.5.12

application.yaml

micronaut:
  server:
    port: 9090
    thread-selection: io

Method Signature

@Post(uris = {
            "/hello"}
    )
    @Produces({MediaType.APPLICATION_JSON})
    @Consumes({MediaType.ALL})
    @ExecuteOn(TaskExecutors.IO)  public MutableHttpResponse<String> stream(HttpRequest httpRequest, @Body @Nullable InputStream payload) throws IOException {
....
}

Actual Behaviour

When using large payloads say approx 5 mb, body read Happen on NIO thread pool blocking the event loop. This cause stuck threads in our application.

application.yaml

micronaut:
  server:
    port: 9090
    thread-selection: io

Method Signature

@Post(uris = {
            "/hello"}
    )
    @Produces({MediaType.APPLICATION_JSON})
    @Consumes({MediaType.ALL})
    @ExecuteOn(TaskExecutors.IO)  public MutableHttpResponse<String> stream(HttpRequest httpRequest, @Body @Nullable InputStream payload) throws IOException {
....
}

Steps To Reproduce

No response

Environment Information

No response

Example Application

No response

Version

2.5.5, 2.5.12

@sumitaneja
Copy link
Author

reproducer.zip

@sumitaneja
Copy link
Author

To reproduce run PayloadInputStreamSpec. In logs can grep on "Server received streaming message " for thread pool used.

@graemerocher graemerocher added status: example attached priority: high High priority type: bug Something isn't working labels Sep 6, 2021
@graemerocher graemerocher added this to the 2.5.14 milestone Sep 6, 2021
@graemerocher graemerocher added this to High Priority Issues in Micronaut Developers Work Coordination Sep 6, 2021
@graemerocher graemerocher removed the type: bug Something isn't working label Sep 6, 2021
@graemerocher
Copy link
Contributor

how is this manifesting as a problem? The event loop is indeed used to read the data but that is correct, do you have an example that demonstrates the stuck threads problem described?

@jameskleeh
Copy link
Contributor

The thread where that message is printed is not the same as the tread where a block occurs waiting for the data.

This seems related to the logging of the data. I commented out # log-level: DEBUG and the test passes.

@sumitaneja
Copy link
Author

@graemerocher In our application we have readiness, liveliness enabled and multiple http client calls on every request.

What we have seen is with inputstream as body request get stuck on http client call (if body is not read).

If we change body to byte[] or read it from stream before making client call, it works.
Trying to create simple reproducer for this. Will attach thread dump to for stuck thread scenario.

From thread dump in stuck thread case we can see one of the eventloop thread is TIMED_WAITING state waiting for body to be read.

@jameskleeh
Copy link
Contributor

@sumitaneja If you aren't reading the body then the Netty thread will have to wait because it doesn't buffer. If you want buffering or other advanced behavior then use a reactive stream

@sumitaneja
Copy link
Author

@jameskleeh logging was enabled for diagnostic purposes to see on which thread body was being read.

@jameskleeh jameskleeh added closed: notabug The issue is not a bug and removed priority: high High priority labels Sep 6, 2021
@jameskleeh jameskleeh removed this from High Priority Issues in Micronaut Developers Work Coordination Sep 6, 2021
@jameskleeh jameskleeh removed this from the 2.5.14 milestone Sep 6, 2021
@sumitaneja
Copy link
Author

@jameskleeh we need to invoke another library which requires inputstream. And content can be as large as 1 GB. so byte array is not an option.
can you point to documentation/example for what you have mentioned above.

@graemerocher
Copy link
Contributor

@sumitaneja please provider a reproducer. Thanks.

@jameskleeh
Copy link
Contributor

@jameskleeh jameskleeh changed the title @Body InputStream Blocks nioEventloop for large payloads @Body InputStream Blocks nioEventloop when its not read Sep 6, 2021
@sumitaneja
Copy link
Author

From micronaut 2.5.5. documentation:

Core Features
IO Stream Support
It is now possible to define a body argument of a controller method with an InputStream. For example @Body InputStream inputStream. Note that you must offload the execution to another thread pool to avoid blocking the event loop when reading the stream. InputStream can also be returned from controller methods. The stream will be read by Micronaut on the IO thread pool if the controller method is executed on the event loop.

@jameskleeh
Copy link
Contributor

@sumitaneja That is referring to input streams returned from controllers, however its also not the case but it once was. Thanks for letting us know.

@sumitaneja
Copy link
Author

sumitaneja commented Sep 7, 2021

reproducer.zip

Updated reproducer with blocking threads, IN zip resources under test also contain thread dump.

@graemerocher graemerocher removed the closed: notabug The issue is not a bug label Sep 7, 2021
@jameskleeh
Copy link
Contributor

The test passes for me. Ran it several times

@sumitaneja
Copy link
Author

This might have some relation with default nioeventloop size, its consistent on my box. In Spec file you can change max to 20 to increase clients.

@graemerocher
Copy link
Contributor

I can indeed reproduce with a value of 30. Thanks for the reproducer.

@graemerocher graemerocher added the type: bug Something isn't working label Sep 8, 2021
@graemerocher
Copy link
Contributor

graemerocher commented Sep 8, 2021

So yeah the input stream is indeed blocking the event loop, I think we need to run the reading of the body data on a different thread. However there is a workaround for your use case since the root cause is that you are using blocking operations on the client. If you want to do this you should configure a different thread pool for client requests like:

micronaut:
  netty:
    event-loops:
      client-loop:
        num-threads: 10  # or whatever is appropriate for your app and the load it handles
        prefer-native-transport: true
  http:
    client:
      read-timeout: 300s
      event-loop-group: client-loop

@graemerocher
Copy link
Contributor

With the above change to the configuration the application behaves correctly although the input stream reading is still happening on the server event loop which may lead to contention so I think we still need to investigate that.

@graemerocher graemerocher self-assigned this Sep 8, 2021
@graemerocher graemerocher added the status: pr submitted A pull request has been submitted for the issue label Sep 8, 2021
jameskleeh added a commit that referenced this issue Sep 9, 2021
* Don't block the event loop when reading inputstream. Fixes #6100

* Use only publishOn

* add issue link

* synchronize access to input stream

* Lazily init processor

* Skip offloading to IO

Co-authored-by: jameskleeh <james.kleeh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: example attached status: pr submitted A pull request has been submitted for the issue type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants