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

Add maxEncodeSize configuration for zstd compression #11361

Merged

Conversation

glorrian
Copy link
Contributor

As mentioned in #11343, Zstd encoder in Netty has a unique parameter of maxEncoded size, and if users of Micronaut want to use Zstd with files more than ≈31Mb Netty will throw an exception like "requested encode buffer size exceeds the maximum allowable size) so I thing we should enable change this parameters for MK's users if they would like to handle bigger files at MK server

@glorrian glorrian marked this pull request as ready for review November 20, 2024 22:31
@glorrian
Copy link
Contributor Author

@yawkat @graemerocher @dstepanov I hope that I did everything right and I will be glad to receive a review. I can't write tests because its need to add a library https://github.com/luben/zstd-jni and i can't do it, because it's not in netty-bom and I can't connect it transitively because it's declared with optional=true in netty poms, I'm not sure about its license, so I didn't add it to libs.versions.toml

/**
* @return The maximum size of data that can be encoded using the zstd algorithm.
*/
int getMaxZstdEncodeSize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a default implementation for compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it already there

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test?

@glorrian
Copy link
Contributor Author

Could we add a test?

I'm not sure about this dependency, but formally we can

I can't write tests because its need to add a library https://github.com/luben/zstd-jni and i can't do it, because it's not in netty-bom and I can't connect it transitively because it's declared with optional=true in netty poms, I'm not sure about its license, so I didn't add it to libs.versions.toml

@glorrian
Copy link
Contributor Author

Also i've tested it manually and it works on my local but i didn't run any auto tests

@glorrian
Copy link
Contributor Author

@sdelamo

@graemerocher graemerocher merged commit 39584d2 into micronaut-projects:4.7.x Nov 22, 2024
16 checks passed
@graemerocher graemerocher added the type: improvement A minor improvement to an existing feature label Nov 22, 2024
@glorrian glorrian deleted the zstd-max-encoded-size-netty branch November 22, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants