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

Allow selection of Heap or Direct buffers for CommandHandler.buffer #1223

Closed
dantheperson opened this issue Feb 10, 2020 · 1 comment
Closed

Allow selection of Heap or Direct buffers for CommandHandler.buffer #1223

dantheperson opened this issue Feb 10, 2020 · 1 comment
Milestone

Comments

@dantheperson
Copy link

@dantheperson dantheperson commented Feb 10, 2020

Feature Request

Is your feature request related to a problem? Please describe

On my application during load spikes the command CommandHandler.stack grows to 250ish command and the buffer grows up to 1.5Gb. (when redis data values being returned are only Kbs in size)
heap buffers being visible in heap dumps, aid in diagnosis of memory issues

Describe the solution you'd like

If you used alloc.buffer(...) instead of alloc.directBuffer(...) it would allow the user to choose between heap or direct buffers with -Dio.netty.noPreferDirect=true

@mp911de mp911de added this to the 5.3.0 milestone Feb 11, 2020
mp911de added a commit that referenced this issue Feb 11, 2020


Lettuce now respects PlatformDependent.directBufferPreferred() when allocating buffers so buffers are either all direct or heap buffers.
mp911de added a commit that referenced this issue Feb 11, 2020


Lettuce now respects PlatformDependent.directBufferPreferred() when allocating buffers so buffers are either all direct or heap buffers.
@mp911de

This comment has been minimized.

Copy link
Member

@mp911de mp911de commented Feb 11, 2020

That's in place now. Care to give Lettuce 5.3.0.BUILD-SNAPSHOT a try? The aggregation buffer in CommandHandler isn't bounded. It could make sense to add an upper bound so that the fill ratio is hit earlier. On the other side, a bound can easily break applications that work with large values.

@mp911de mp911de closed this Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.