fix: reduce memory usage in the mailserver#7523
Merged
Merged
Conversation
Part status-im/status-app#21142 - Reduce resume-time backend pressure by throttling historic sync requests in messenger_mailserver.go. - Keep functional behavior intact: still request historic messages on resume/availability triggers, but avoid repeated bursts. - Add mutex-guarded sync gate on `RequestAllHistoricMessages`: - skip when another historic sync is already in progress - skip when the previous request is within `historicSyncMinInterval` - clear in-flight state with deferred unlock path - Place throttling after existing sync preconditions (`shouldSync`, `mailserversDatabase`), so startup early-exit paths do not consume the throttle window.
✅ Companion PR Verified |
Member
Jenkins BuildsClick to see older builds (10)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/10.34.x #7523 +/- ##
==================================================
Coverage ? 61.27%
==================================================
Files ? 850
Lines ? 119093
Branches ? 0
==================================================
Hits ? 72969
Misses ? 38652
Partials ? 7472
Flags with carried forward coverage won't be shown. Click here to find out more.
|
… needed - Applied a minimal, low-risk optimization to reduce backend memory usage during background historic sync. - Kept existing public behavior unchanged for normal/API-triggered sync calls. - Added an internal path that skips building and merging MessengerResponse payloads when results are not needed. - Wired the async background trigger to use that non-aggregating path.
Member
Author
|
I added a second commit that's a quick win to reduce background memory usage. See commit description |
There was a problem hiding this comment.
Pull request overview
This PR reduces backend pressure by preventing overlapping and overly-frequent historic message sync requests, primarily around resume/store-node availability triggers in the Messenger mailserver sync flow.
Changes:
- Add a mutex-guarded in-flight flag and minimum interval tracking to gate historic sync requests.
- Refactor
RequestAllHistoricMessagesinto an internal helper that can optionally skip aggregating responses (used by async calls). - Update the async historic sync path to call the internal helper without aggregation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
protocol/messenger.go |
Adds state (mutex, in-flight flag, last request timestamp) to support throttling/gating historic sync requests. |
protocol/messenger_mailserver.go |
Implements the throttle + in-flight gate and refactors historic sync entrypoints to support optional response aggregation. |
protocol/messenger_mailserver_cycle.go |
Switches async historic sync to call the new internal helper without aggregating responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
And a final small change that should help a little bit by pre-sizing the maps to the right size |
jrainville
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part status-im/status-app#21142
Reduce resume-time backend pressure by throttling historic sync requests in messenger_mailserver.go.
Keep functional behavior intact: still request historic messages on resume/availability triggers, but avoid repeated bursts.
Add mutex-guarded sync gate on
RequestAllHistoricMessages:historicSyncMinIntervalPlace throttling after existing sync preconditions (
shouldSync,mailserversDatabase), so startup early-exit paths do not consume the throttle window.Applied a minimal, low-risk optimization to reduce backend memory usage during background historic sync.
Kept existing public behavior unchanged for normal/API-triggered sync calls.
Added an internal path that skips building and merging MessengerResponse payloads when results are not needed.
Wired the async background trigger to use that non-aggregating path.
status-app PR: status-im/status-app#21159