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

MaxChunkSize not taken into account for Quarkus Reasteasy Reactive client #37786

Closed
fredericBregier opened this issue Dec 16, 2023 · 7 comments · Fixed by #37830
Closed

MaxChunkSize not taken into account for Quarkus Reasteasy Reactive client #37786

fredericBregier opened this issue Dec 16, 2023 · 7 comments · Fixed by #37830
Labels
area/rest-client kind/bug Something isn't working
Milestone

Comments

@fredericBregier
Copy link

fredericBregier commented Dec 16, 2023

Describe the bug

According to the documentation, setting the following should change the chunk size for the client.

quarkus.resteasy-reactive.output-buffer-size=65536
quarkus.resteasy-reactive.input-buffer-size=65536
quarkus.http.limits.max-chunk-size=65536

Howver, during my tests, using a Netty server to trace and then going in debug mode, the reactive client uses 8KB chunk size, whatever the configuration. Following the code, I think I discover that the current client is taking its configuration from (defined in QuarkusRestClientProperties):

"io.quarkus.rest.client.max-chunk-size"

Which is possibly (not sure) associated with non reactive clients (not validated by Quarkus if not getting non reactive clients into the pom):

quarkus.rest-client.multipart.max-chunk-size

I didn't find a way to define this value in the properties file.

It seems, according to my search, that the following are intend for Server side mainly.

quarkus.resteasy-reactive.output-buffer-size=65536
quarkus.resteasy-reactive.input-buffer-size=65536
quarkus.http.limits.max-chunk-size=65536

I found a way to set this value by using the programmatic builder as the following:

QuarkusRestClientBuilder.newBuilder().baseUri(uri)
    .property("io.quarkus.rest.client.max-chunk-size",65536).build(apiClass);

I believe something is missing in the configuration that should take at least the quarkus.http.limits.max-chunk-size to set this value io.quarkus.rest.client.max-chunk-size for the resteasy reactive client.

Side not : it seems the reactive server side is honoring those configuration, even if for an unknown reason it is not possible to go further than 64 KB (even if I set 128KB, the chunk size remains at 64 KB). But the main issue there is on reactive client side.

Expected behavior

When setting quarkus.http.limits.max-chunk-size, the corresponding value io.quarkus.rest.client.max-chunk-size for the resteasy reactive client (or other ways to do it) should be set such that Resteasy reactive client uses the correct chunk size instead of default 8K.

Actual behavior

Ignore all settings, except in programmatic building way by passing the internal property io.quarkus.rest.client.max-chunk-size.

How to Reproduce?

A simple client sending an array of bytes greater than 64KB to a server in reactive resteasy mode with configuration as described in the documentation will ignore the chunk size (while server in response seems to honore it).

Output of uname -a or ver

6.5.0-14-generic #14-Ubuntu SMP PREEMPT_DYNAMIC Tue Nov 14 14:59:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "21.0.1" 2023-10-17 LTS OpenJDK Runtime Environment Temurin-21.0.1+12 (build 21.0.1+12-LTS) OpenJDK 64-Bit Server VM Temurin-21.0.1+12 (build 21.0.1+12-LTS, mixed mode, sharing)

Quarkus version or git rev

3.6.3

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.7 Maven home: /usr/share/maven Java version: 21.0.1, vendor: Eclipse Adoptium, runtime: /usr/lib/jvm/temurin-21-jdk-amd64 Default locale: fr_FR, platform encoding: UTF-8 OS name: "linux", version: "6.5.0-14-generic", arch: "amd64", family: "unix"

Additional information

No response

Copy link

quarkus-bot bot commented Dec 18, 2023

/cc @cescoffier (rest-client)

@geoand
Copy link
Contributor

geoand commented Dec 18, 2023

Any chance you can attach a sample showing the behavior you describe?

Thanks

@fredericBregier
Copy link
Author

@geoand Yes, I try. See here: https://github.com/fredericBregier/clientwithchunk

Note that I must precise the bug: this impacts merely the Programmatic client, not the "Injected" version

  • Using "injected" client seems to work (up to 64KB, not more)
  • Using Programmatic client does not worked (8KB limit) except if specific configuration is passed at build step
    • Still the limit is then 64 KB

I'm using "programmatic client" since I cannot Inject client where I need it (those parts of code are out of @ApplicationScoped so I need to use a constructor, so the Programmatic builder).

It seems there is a limit up to 64KB, both client and server side (POST vs GET), but that's less important, even if I would appreciate to go further.

Having the possibility to increase the chunk size is greatly appreciated when sending large InputStream (see my PR to handle this from client side).

So the first step, is to be able to specify this

@geoand
Copy link
Contributor

geoand commented Dec 19, 2023

Thanks!

@geoand
Copy link
Contributor

geoand commented Dec 19, 2023

A few comments:

  • quarkus.http.limits.max-chunk-size is definitely not supposed to be taken into account by the client, and that is not going to be changed.
  • There is a indeed a bug in the code that results in max-chunk-size not being taken into account when using the programmatic client.
  • The name of the max-size-property property is not great.

I have opened #37830 as a response to this issue.

@fredericBregier
Copy link
Author

fredericBregier commented Dec 19, 2023

Thanks !
I understand that quarkus.http.limits.max-chunk-size property is in fact for server side (not that clear in documentation).

The property quarkus.rest-client.multipart.max-chunk-size was not clear indeed but didn't work whatever (in the example, I tried but with no success).

And what about quarkus.resteasy-reactive.output-buffer-size or quarkus.resteasy-reactive.input-buffer-size ? Again only for server side ?

However, I'll wait for your patch hopefully ! Great thank you !

@geoand
Copy link
Contributor

geoand commented Dec 19, 2023

Yes, the properties you mention are also only server side

geoand added a commit to geoand/quarkus that referenced this issue Dec 20, 2023
geoand added a commit that referenced this issue Dec 20, 2023
Take max-chunk-size config into account for programmatic client
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest-client kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants