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

ProxyFactory#getProxy seems to re-use EVM instances from the cache without properly settings optionalParameters #1805

Closed
EugenMayer opened this issue Sep 28, 2023 · 6 comments · Fixed by #1806
Assignees
Labels
component: entity-view kind: bug worth: high Implementing this has a high worth
Milestone

Comments

@EugenMayer
Copy link
Contributor

Assuming that one uses entityViewConfiguration.addOptionalParameters to insert default parameters for an EVM to carry (specifically beans on needs to use during Pre/Post remove listeners), it seems that during the invocation attempt for thus listeners, and EVM instance is created via context.getEntityViewManager().getSerializableDelegate(view.$$_getEntityViewClass())
which ultimately will use the ProxyFactory to create or *reuse an EVM.

When an EVM is "reused" it is not ensured, that this EVM has the optionalParameters applied and thus we can end up having an EVM without those parameters included.

Mobe91 added a commit to Mobe91/blaze-persistence that referenced this issue Sep 28, 2023
Mobe91 added a commit to Mobe91/blaze-persistence that referenced this issue Sep 29, 2023
Mobe91 added a commit to Mobe91/blaze-persistence that referenced this issue Oct 1, 2023
@Mobe91
Copy link
Contributor

Mobe91 commented Oct 4, 2023

@EugenMayer would you be able to try if the changes in #1806 solve this?

@beikov beikov added this to the 1.6.10 milestone Oct 4, 2023
@beikov beikov added kind: bug component: entity-view worth: high Implementing this has a high worth labels Oct 4, 2023
@EugenMayer
Copy link
Contributor Author

@Mobe91 thank you a lot for working on this item! Will try to queue the test today

@EugenMayer
Copy link
Contributor Author

@Mobe91 indeed this solves my issue entirely. Did it solve your memory leak issue too?

@Mobe91
Copy link
Contributor

Mobe91 commented Oct 4, 2023

Did it solve your memory leak issue too?

Unfortunately not, there must be other things going on in my case that are probably not related to blaze-persistence 🥲

Glad to hear that it works for you! @beikov will make sure it goes into the upcoming release.

@EugenMayer
Copy link
Contributor Author

@Mobe91 AFAIK you are using spring boot in your tests right?

Some aspects we use:

  1. I'am sure you are aware about the context cache and the ability to limit the amount of caches spring boot is preserving during the tests? For us, this has limitted the used memory and connections. spring.test.context.cache.maxSize=5 or similar helps our cause.

  2. In addition, you might want to implement an TestExecutionListener that clears the EM L1 cache after every test or test suite.

  3. Also, if you are using gradle you might tune your test task

    maxParallelForks = 1
    systemProperties['junit.jupiter.execution.parallel.enabled'] = false

We use this for all JPA based tests.

@Mobe91
Copy link
Contributor

Mobe91 commented Oct 4, 2023

Thank you for the hints. If you are curious, I've opened a spring-boot issue a while ago for this and included quite some details about what I am observing in my case. Reducing the application context cache size was the first thing I did but strangely the application contexts are still not garbage collected even though they are no longer held by the cache.

My tests run in Maven but no parallelism is involved. I might give the EM clearing a try though! 👍

beikov pushed a commit that referenced this issue Oct 10, 2023
beikov pushed a commit to beikov/blaze-persistence that referenced this issue Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: entity-view kind: bug worth: high Implementing this has a high worth
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants