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

Limit amount of native memory used for NIO buffers #542

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Praveen2112
Copy link
Member

Set the default value of jdk.nio.maxCachedBufferSize to 2MB. If we did not set this JVM would take the entire System memory as the maximumbuffer size and it will result in increase in non-heap memory for the JVM process.

@cla-bot cla-bot bot added the cla-signed label Mar 26, 2019
@Praveen2112
Copy link
Member Author

Praveen2112 commented Mar 26, 2019

@findepi Have added the default value as 2MB.

@findepi
Copy link
Member

findepi commented Mar 26, 2019

@Praveen2112 thanks for the PR.
Please change commit title (the first line) to something like Limit amount of native memory used for NIO buffers.

@martint
Copy link
Member

martint commented Mar 26, 2019

We used to set this to 1MB at FB.

@martint
Copy link
Member

martint commented Mar 26, 2019

if you're OK with this change

I'm ok with it, but in general, we should try to figure out why this is happening. The setting is a bandaid to a problem that can be solved by fixing the calling code to size buffers more carefully.

@Praveen2112 Praveen2112 changed the title Add jvm property to set the cached buffer size Limit amount of native memory used for NIO buffers Mar 27, 2019
@Praveen2112
Copy link
Member Author

@findepi Have updated the commit message.
@martint , @findepi Currently we have a check for the Page size when pushing to the OutputBuffer (IIRC the max is 1MB) but we didn't have any such limits for the ConnectorSplit so if we add some restrictions on the split size then we could have a control over the buffer size and usage of native memory.

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Set the default value of jdk.nio.maxCachedBufferSize to 2MB. If we did
not set this JVM would take the entire System memory as the maximum
buffer size and it will result in increase in non-heap memory for the
JVM process
@Praveen2112
Copy link
Member Author

@findepi Thanks for pointing it out. I have updated it.

@findepi findepi merged commit d127b2e into trinodb:master Apr 1, 2019
@findepi findepi mentioned this pull request Apr 1, 2019
6 tasks
@electrum electrum added this to the 307 milestone Apr 4, 2019
@Praveen2112 Praveen2112 deleted the rpm_changes branch April 30, 2019 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants