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

BUG: Memory-leak on streaming queries when LoadBuffers are not aligned #2936

Merged
merged 2 commits into from
Feb 13, 2023

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Jan 18, 2023

Hello Rob, unfortunately, we discovered a second problem with memory leaks in streaming queries.

This happens, when LoadBuffers are involved.

Normally the loadBuffer holds references up to batchSize (default=10) beans. The beans itself holds references to BeanLists, which holds references to the load buffers. This is a cycle, but normally not critical, as there is a cut every batch.

Bean    1 2 3 4 5 6 7 8 ...
BufferA |- 1 -| |- 2 -|
BufferB |- 1 -| |- 2 -|

Bean    1 2 3 4 5 6 7 8 ...
BufferA |- 1 -| |- 2 -| |- 2 -|
BufferB |-1-| |- 2 -| |- 3 -|

If multiple load buffers are involved, we have luck, as long there are correctly aligned and do not "overlap". This overlap may happen if beans have different layouts (e.g. inherited beans)

Bean    1 2 3 4 5 6 7 8 ...
BufferA |- 1 -| |- 2 -|
BufferB |- 1 -| |- 2 -|

Bean    1 2 3 4 5 6 7 8 ...
BufferA |- 1 -| |- 2 -| |- 2 -|
BufferB |-1-| |- 2 -| |- 3 -|

In the test case, I've created ONE TestModel3A bean here, that has the "many2" property missing.
So when the first "findEach" loads the bean, the many1 property has a buffer of 10 belonging to the beans BBBBBABBB and all of them refer back to the same load context. But in many2 there are 10 A beans and the last bean holds a reference to a different load-context. They all build a "chain", so that they cannot be gc'ed.

I've written a test, that fails with -Xmx100m in findEach

@rbygrave How do you think should we fix these issues. Should we introduce weak references in loadContext or do you see other options like skipping the non existent property many2 in TestModel3A, so that the loadcontext will be properly aligned

@rPraml rPraml force-pushed the mem-leak-streaming-queries branch from 0e55f7c to 61d6ccc Compare January 20, 2023 15:51
@rbygrave
Copy link
Member

rbygrave commented Feb 1, 2023

This change looks good to me.

see other options like skipping the non existent property many2 in TestModel3A

I'm not sure about other options. My gut says use this weak references approach as you have done here and to me that sounds simpler and likely more robust.

@rbygrave rbygrave added this to the 13.13.0 milestone Feb 13, 2023
@rbygrave rbygrave added the bug label Feb 13, 2023
@rbygrave rbygrave merged commit 381ac58 into ebean-orm:master Feb 13, 2023
@rPraml rPraml deleted the mem-leak-streaming-queries branch August 10, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants