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

Expose ByteBufAllocator in ClientResources #345

Closed
wants to merge 1 commit into from
Closed

Expose ByteBufAllocator in ClientResources #345

wants to merge 1 commit into from

Conversation

jwils
Copy link

@jwils jwils commented Aug 28, 2016

To have more control over buffer allocation configuration, it would be
nice to include the option to choose what allocator to use.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 92.658% when pulling 59d7add on jwils:expose-allocator into c8c59ba on mp911de:master.

To have more control over buffer allocation configuration, it would be
nice to include the option to choose the allocator to use.
@coveralls
Copy link

coveralls commented Aug 28, 2016

Coverage Status

Coverage decreased (-0.006%) to 92.844% when pulling acbfc81 on jwils:expose-allocator into c8c59ba on mp911de:master.

@mp911de
Copy link
Collaborator

mp911de commented Aug 28, 2016

Hi @jwils thanks for the PR. The code looks pretty decent. Could you help me understand your motivation for the change? Could this issue be solved by switching to ByteBufAllocator.DEFAULT?

@jwils
Copy link
Author

jwils commented Aug 28, 2016

If including netty from lettuce as a shaded jar it'll have to use a different PooledAllocator than other netty instance. The default allocator is configured to use up to 50% of memory so two or more instances can cause problem.

I can use the system properties to set each size if you want to minimize the parameters for ClientResources.

@mp911de
Copy link
Collaborator

mp911de commented Aug 28, 2016

Having a solution outside ClientResources would be my preferred solution. Using the shaded jar comes with several drawbacks and imposes constraints on applications. Reuse of netty resources (allocators, event loops) is no longer possible.

@mp911de
Copy link
Collaborator

mp911de commented Sep 16, 2016

Closing this PR for now.

@mp911de mp911de closed this Sep 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants