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

Support request body #13

Open
plexus opened this issue Nov 9, 2019 · 9 comments
Open

Support request body #13

plexus opened this issue Nov 9, 2019 · 9 comments
Labels
enhancement New feature or request

Comments

@plexus
Copy link
Contributor

plexus commented Nov 9, 2019

When doing a POST request the :body in the request map is nil.

  :body (if (.isBlocking exchange) (.getInputStream exchange))
@ikitommi
Copy link
Member

ikitommi commented Nov 9, 2019

Currently, there is no way to lift all threads to worker pool from XNIO pool. Immutant has :dispatch? option for this. We could have just :dispatch. Without first changing the thread pool, the .isBlocking returns false and :body would just be nil with that code.

@ikitommi ikitommi added the enhancement New feature or request label Nov 9, 2019
@kalekale
Copy link
Contributor

Something like this? #16

@ikitommi
Copy link
Member

PR looks good, one post comment (rebased already) + we are not setting the body yet. Maybe do it only when :dispatch is set? For apps running in XNIO pool, still need to figure out a performant way to stream the body in.

@kalekale
Copy link
Contributor

kalekale commented Nov 21, 2019

We are actually setting the body when we are in a blocking thread (:body (if (.isBlocking exchange) (.getInputStream exchange))). Or maybe I misunderstood you.
Do you have ideas on how to handle the body in a non-blocking thread? There is receiveFullBytes(cb) and receivePartialBytes(cb) for getting the body asynchronously.

@ikitommi
Copy link
Member

Simple way would be to invoke the actual ring/routing handler in the callback only after the body has been read in Full - the bytes could be wrapped into ByteArrayInputStream to be Ring-compatible.

There are many ways to optimize that afterwards: if the data is huge, should be handled in chuncks (I guess the Partial thing), if the route doesn't need the body, there is no idea trying to read it etc. Could be a protocol, that for example reitit-pohjavirta implements by peeking at the [:parameters :body] definition etc.

@kalekale
Copy link
Contributor

"invoke the actual ring/routing handler in the callback" but wouldn't this block the IO thread?

@ikitommi
Copy link
Member

It your code is non-blocking, you can run it in the IO Thread. I think there are 4 different ways in this:

  1. go fully non-blocking: read the body as non-blocking, handle the request in same pool
  2. read the body as non-blocking in XNIO pool, lift the actual request handling to worker pool
  3. go blocking, e.g. dispatch the thread to worker, read via stream

@kanwei
Copy link

kanwei commented May 3, 2020

Any update on how to best implement this? I think this is the last issue that needs to be resolved before the library can be used in production. Would be happy to help if the right approach were documented.

@bsless
Copy link

bsless commented May 24, 2021

Hey everyone, any update? It's been a year

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants