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.

Sign up for GitHub

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

Jump to bottom

During a resend-request the request for messages from storage allows for unbounded memory usage #639

Open
philipwhiuk opened this issue Jun 1, 2023 · 3 comments · May be fixed by #643
Open

During a resend-request the request for messages from storage allows for unbounded memory usage #639

philipwhiuk opened this issue Jun 1, 2023 · 3 comments · May be fixed by #643
Labels
bug

Comments

@philipwhiuk
Copy link
Contributor

philipwhiuk commented Jun 1, 2023 •
edited

When a resend request to Infinity (or even just a large capped number) is made, this can cause a huge number of messages to be fetched from storage into memory which can cause the application to crash due to lack of memory. It's not practical to mitigate this in the storage layer

In addition even if they fit in memory without a fix to #271 this can lead to some fairly ugly behaviour where we continue to attempt to send messages to a session that's already disconnected us.

To Reproduce

  • Send several thousand messages (volume necessary dependent on application memory profile)
  • Instruct the counter-party to perform a resend-request of all these messages

Expected behavior
We should fetch the messages in batches and then send them a batch at a time

System information:
N/A

Additional context
I've got a hot fix for some of this - will try to tidy it up and submit for review.

This is probably additionally key if we implement #621
@philipwhiuk philipwhiuk added the bug label Jun 1, 2023
@philipwhiuk philipwhiuk changed the title The request for messages from storage allows for unbounded memory usage During a resend-request the request for messages from storage allows for unbounded memory usage Jun 1, 2023
@chrjohn
Copy link
Member

chrjohn commented Jun 16, 2023

As you noted on #621 (comment) probably it would be a good thing to have a config option to restrict the maximum number of messages that could be resent. The beginning of the range could be skipped over by setting the SequenceReset tag NewSeqNo accordingly.

@philipwhiuk philipwhiuk linked a pull request Jun 16, 2023 that will close this issue
Open
@philipwhiuk
Copy link
Contributor Author

philipwhiuk commented Jun 16, 2023 •
edited

Yeah, a maximum resend request is probably also a good idea. I'd suggest that there's several things you could do:

  1. Logout the counter-party (the session is an invalid state, just like we do when the sequence number is too high
  2. Cap the resend amount to the limit and gap-fill
  3. No limit, use the batching

At some point I also need to look a throttling send - when we fixed our batching (and we really did want to send them all... we effectively DDoSed our counter-party until they caught up - ideally we could have throttled it so they stayed connected).

@chrjohn chrjohn mentioned this issue Aug 16, 2023
Open
@wajncn
Copy link

wajncn commented Mar 20, 2024

#778
Could solve your problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
3 participants
@philipwhiuk @chrjohn @wajncn