Skip to content

ByteBuffer is not released when compression is enabled, causing pool leakage #883

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

Merged
merged 4 commits into from
Mar 3, 2022

Conversation

kulabun
Copy link
Contributor

@kulabun kulabun commented Mar 2, 2022

Hey Mongo team.

My team experienced an increased number of failures caused by latency recently. Further investigation showed that InternalStreamConnection doesn't close buffers returned by PowerOfTwoBufferPool when compression is enabled. It leads to permits leakage. After INTEGER_MAX buffer retrievals buffer pool is fully exhausted and all following requests would be blocked indefinitely unless the thread gets interrupted.

JAVA-4510

@kulabun
Copy link
Contributor Author

kulabun commented Mar 2, 2022

An alternative way to address the issue can be buffer release as part of Compressor.ByteBufInputStream.close() method. For example, SocketInputStream works this way and closes the underlying socket as part of close call. But I think it's better to be explicit and close it within the same section that used it.

@jyemin jyemin requested a review from stIncMale March 2, 2022 14:51
@jyemin jyemin changed the title Bugfix: bytebuffer is not released when compression is enabled causing bytebuffer pool leakage ByteBuffer is not released when compression is enabled causing pool leakage Mar 2, 2022
@jyemin jyemin changed the title ByteBuffer is not released when compression is enabled causing pool leakage ByteBuffer is not released when compression is enabled, causing pool leakage Mar 2, 2022
@stIncMale stIncMale requested review from jyemin and removed request for stIncMale March 3, 2022 00:04
@stIncMale stIncMale self-assigned this Mar 3, 2022
@stIncMale
Copy link
Member

stIncMale commented Mar 3, 2022

Hi @kulabun,

Thank you for the PR. I looked through the class InternalStreamConnection trying to see whether there are other places where we may not release buffers, and pushed a new commit.

Using your code from JAVA-4510 I confirmed that after the changes threads are not blocked due to ConcurrentPool.maxSize being exhausted. This can be done quickly by changing the ConcurrentPool.INFINITE_SIZE constant from Integer.MAX_VALUE to something small.

@stIncMale stIncMale requested a review from jyemin March 3, 2022 02:33
Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

Added a comment.

@stIncMale stIncMale requested a review from jyemin March 3, 2022 17:04
Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM

@stIncMale stIncMale merged commit d89ed7e into mongodb:master Mar 3, 2022
stIncMale added a commit to stIncMale/mongo-java-driver that referenced this pull request Mar 3, 2022
…leakage (mongodb#883)

JAVA-4510

Co-authored-by: Konstantin Labun <konstantinlabun@Konstantins-Air.Home>
Co-authored-by: Valentin Kovalenko <valentin.kovalenko@mongodb.com>
stIncMale added a commit to stIncMale/mongo-java-driver that referenced this pull request Mar 3, 2022
…leakage (mongodb#883)

JAVA-4510

Co-authored-by: Konstantin Labun <konstantinlabun@Konstantins-Air.Home>
Co-authored-by: Valentin Kovalenko <valentin.kovalenko@mongodb.com>
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.

3 participants