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

Memory Leak when using Multi Replay #1482

Closed
markusdlugi opened this issue Jan 9, 2024 · 6 comments · Fixed by #1483
Closed

Memory Leak when using Multi Replay #1482

markusdlugi opened this issue Jan 9, 2024 · 6 comments · Fixed by #1483
Labels
bug Something isn't working
Milestone

Comments

@markusdlugi
Copy link
Contributor

Context

In one of our applications we observed a memory leak when using Multi.createBy().replaying() due to subscriptions not being removed properly. After removing the usage, the memory leak vanished.

Description

We were using the Multi replay functionality in one of our applications to achieve that a REST call is only performed once and the result then re-used for all subsequent subscribers. The code looked something like this:

Multi.createBy().replaying().upTo(1).ofMulti(performRestCallAndStuff()).collect().first();

The resulting Uni would be consumed each time a message was received via SQS, i.e., each message resulted in a new subscription.

During some load tests where thousands of messages were sent to the application, we observed that the application's memory usage would increase continuously until the application ultimately crashed with OOM. After some debugging, we identified the Multi replay as the root cause. After removing it and replacing it with a different caching mechanism, the application's memory usage is fine again.

Additional details

We debugged further and found the culprit to be the subscriptions list in the ReplayOperator class. We observed during debugging that the size of the list keeps on increasing and no subscriptions are ever removed. We believe this is caused by ReplayOperator#subscribe calling subscriber.onSubscribe(replaySubscription) before subscriptions.add(replaySubscription). The former would result in cancel() to be called on the ReplaySubscription (and thus also subscriptions.remove(this)) even before the subscription was added to the list. So in the end, the subscription was not removed and stayed in the list forever.

  • Quarkus version: 3.6.4
  • Mutiny version: 2.5.1
@jponge
Copy link
Member

jponge commented Jan 9, 2024

I'll have a look, probably next week.

However since you've done a good amount of investigation, would you be willing to prepare a pull-request?

markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 9, 2024
Fixes a memory leak caused by old subscriptions not being removed properly. In addition, this fixes the edge case where no completion event would be sent for replay subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 9, 2024
Fixes a memory leak caused by old subscriptions not being removed properly. In addition, this fixes the edge case where no completion event would be sent for replay subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 9, 2024
Fixes a memory leak caused by old subscriptions not being removed properly. In addition, this fixes the edge case where no completion event would be sent for replay subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
@markusdlugi
Copy link
Contributor Author

@jponge, I created a PR, would be great if you could have a look. As expected, the fix itself was not too troublesome, but a proper test was a little more challenging :)

@jponge
Copy link
Member

jponge commented Jan 9, 2024 via email

markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 10, 2024
Fixes a memory leak caused by old subscriptions not being removed properly.
In addition, this fixes the edge case where no completion event would be sent for replay
subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 11, 2024
Fixes a memory leak caused by old subscriptions not being removed properly.
In addition, this fixes the edge case where no completion event would be sent for replay
subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 11, 2024
Fixes a memory leak caused by old subscriptions not being removed properly.
In addition, this fixes the edge case where no completion event would be sent for replay
subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
@jponge jponge added this to the 2.5.4 milestone Jan 12, 2024
@jponge jponge added this to Backlog in Mutiny development via automation Jan 12, 2024
@jponge jponge added the bug Something isn't working label Jan 12, 2024
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 12, 2024
Fixes a memory leak caused by old subscriptions not being removed properly.
In addition, this fixes the edge case where no completion event would be sent for replay
subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
markusdlugi added a commit to markusdlugi/smallrye-mutiny that referenced this issue Jan 12, 2024
Fixes a memory leak caused by old subscriptions not being removed properly.
In addition, this fixes the edge case where no completion event would be sent for replay
subscriptions if the number of requested items was exactly equal to the number of available items.

Fixes smallrye#1482
Mutiny development automation moved this from Backlog to Done Jan 12, 2024
@markusdlugi
Copy link
Contributor Author

@jponge do you plan to create a new release for this fix so it can be incorporated in the next Quarkus version?

@jponge
Copy link
Member

jponge commented Jan 15, 2024 via email

@jponge
Copy link
Member

jponge commented Jan 19, 2024

@markusdlugi 2.5.4 has been released. It will eventually land in Quarkus, but meanwhile you can manually add an explicit dependency to Mutiny 2.5.4 in your Quarkus projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants