feat: pause background work when app is in background#7373
feat: pause background work when app is in background#7373
Conversation
|
alexjba
changed the title
Jenkins BuildsClick to see older builds (102)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7373 +/- ##
===========================================
- Coverage 61.46% 61.38% -0.09%
===========================================
Files 824 825 +1
Lines 116134 116640 +506
===========================================
+ Hits 71386 71594 +208
- Misses 37498 37764 +266
- Partials 7250 7282 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
|
alexjba
force-pushed
the
feat/reduce-app-work
branch
2 times, most recently
from
77a9572 to
830ab56
Compare
alexjba
force-pushed
the
feat/local-notifications
branch
from
9e77ec1 to
7cc373b
Compare
alexjba
force-pushed
the
feat/reduce-app-work
branch
from
830ab56 to
2715c40
Compare
There was a problem hiding this comment.
Pull request overview
Adds a lifecycle-driven pause/resume mechanism across backend, node services, messaging, and network subsystems to reduce non-essential work while the app is backgrounded (battery/resource savings), plus a couple of related operational/UX improvements.
Changes:
- Introduce
pkg/messaging/lifecyclepaused-background pub/sub and integrate it across multiple polling loops/tickers (waku, transport, envelopes monitor, backups, NTP, rpc limiter, IPFS, etc.). - Add backend/node/service pause/resume hooks (
AppStateChange-driven pause/play;StatusNode.PauseBackground/ResumeForeground; service-level implementations/tests). - Improve ancillary behavior: richer local notification previews and media server port reuse across foreground/background cycles; RPC client limiter cleanup on stop; simplify mobile AppStateChange API.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/wallet/service_pause_play_test.go | Adds wallet service pause/resume noop tests. |
| services/wallet/service.go | Adds wallet pause/resume state + locking and worker start/stop helpers. |
| services/wallet/reader.go | Adds Reader.IsRunning() to support pause/resume decisions. |
| services/newsfeed/service_test.go | Adds newsfeed pause/resume test. |
| services/newsfeed/service.go | Implements pause/resume via Stop/Start + tracks started. |
| services/ext/service_test.go | Tests ext service pause/resume behavior and lifecycle flag updates. |
| services/ext/service.go | Implements pause/resume by toggling messenger paused state. |
| services/connector/service_test.go | Adds connector pause/resume test (includes timing wait). |
| services/connector/service.go | Adds pause/resume via stop/start with mutex + state flags. |
| services/backup/controller_test.go | Adds backup controller lifecycle pause/resume test. |
| services/backup/controller.go | Makes backup ticker respect paused-background lifecycle subscription. |
| server/server.go | Adds lifecycle locking, cached port reuse, and retry logic for foreground start. |
| server/pairing/server.go | Switches pairing BaseServer to embed *server.Server (pointer). |
| server/pairing/connection_test.go | Updates pairing test wiring to match pointer-embedded server. |
| server/lifecycle_test.go | Tests cached-port binding and port reuse across background/foreground. |
| protocol/messenger_status_updates.go | Skips periodic status broadcasts when paused in background. |
| protocol/messenger_raw_message_resend.go | Skips resend loop work while paused. |
| protocol/messenger_pause_play_test.go | Tests messenger paused flag updates lifecycle/global state. |
| protocol/messenger_mailserver.go | Skips missing-message checks while paused. |
| protocol/messenger_curated_communities.go | Skips curated community updates while paused. |
| protocol/messenger_communities.go | Skips multiple community periodic loops while paused. |
| protocol/messenger.go | Adds atomic paused flag + SetPausedBackground, hooks ToForeground/ToBackground, propagates pause to lifecycle/datasync/push client. |
| protocol/local_notifications_test.go | Adds unit tests for message preview extraction and timestamp fallback. |
| protocol/local_notifications.go | Uses safer message preview extraction; avoids nil timestamp panics; improves notification bodies. |
| protocol/communities/manager_test.go | Adds archive interval test for paused lifecycle. |
| protocol/communities/manager_archive.go | Makes archive scheduling/download polling respect paused lifecycle. |
| pkg/messaging/waku/nwaku.go | Pauses connection-change ticker loop via lifecycle subscription. |
| pkg/messaging/waku/gowaku.go | Pauses multiple waku loops; introduces reduced peer exchange behavior while paused. |
| pkg/messaging/waku/api_test.go | Adds tests for shared paused polling loop helper. |
| pkg/messaging/waku/api.go | Adds lifecycle-aware polling helper for RPC message subscription polling. |
| pkg/messaging/lifecycle/background_mode_test.go | Adds tests for paused-background pub/sub semantics and concurrency. |
| pkg/messaging/lifecycle/background_mode.go | Implements global paused flag + subscriber management with coalescing. |
| pkg/messaging/layers/transport/transport_test.go | Tests transport filter cleanup loop pause/resume behavior. |
| pkg/messaging/layers/transport/transport.go | Adds lifecycle-aware filter cleanup loop + test hook interval/fn. |
| pkg/messaging/layers/transport/envelopes_monitor_test.go | Tests envelopes retry loop skipping when paused. |
| pkg/messaging/layers/transport/envelopes_monitor.go | Makes envelopes retry loop lifecycle-aware. |
| pkg/messaging/layers/reliability/datasync/transport.go | Adds paused-background control over datasync tick rate + uses dynamic offset in backoff calculation. |
| pkg/backend/pre_login.log | Adds a pre-login log artifact file. |
| pkg/backend/pause_play_test.go | Adds backend lifecycle/AppStateChange tests for paused/stopped transitions. |
| pkg/backend/pause_play.go | Introduces backend lifecycle state machine + pause/resume orchestration. |
| pkg/backend/node/pause_play_test.go | Tests StatusNode pause/resume calls services implementing lifecycle hooks. |
| pkg/backend/node/get_status_node.go | Adds background lifecycle interface + PauseBackground/ResumeForeground on StatusNode. |
| pkg/backend/geth_backend.go | Integrates pause/resume into AppStateChange; adds lifecycle locking and state transitions. |
| mobile/status.go | Simplifies mobile AppStateChange API to single call returning JSON response; removes V2. |
| mobile/requests/app_state_change_test.go | Removes request validation tests (AppStateChangeV2 removal). |
| mobile/requests/app_state_change.go | Removes request schema/validator for AppStateChangeV2. |
| internal/timesource/ntp_test.go | Tests NTP periodic runner pausing/resuming via lifecycle. |
| internal/timesource/ntp.go | Makes NTP periodic runner lifecycle-aware (timer-based). |
| internal/rpc/client.go | Stops and clears per-provider limiters during client Stop. |
| internal/rpc/chain/rpclimiter/rpc_limiter_test.go | Adds lifecycle pause/resume test for RPC limiter release loop. |
| internal/rpc/chain/rpclimiter/rpc_limiter.go | Pauses limiter ticker loop via lifecycle subscription. |
| internal/ipfs/ipfs_test.go | Tests IPFS downloader dispatcher pausing/resuming via lifecycle. |
| internal/ipfs/ipfs.go | Makes IPFS task dispatcher lifecycle-aware. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
alexjba
force-pushed
the
feat/local-notifications
branch
2 times, most recently
from
9bfce9b to
471c25e
Compare
alexjba
force-pushed
the
feat/reduce-app-work
branch
4 times, most recently
from
dcc1af8 to
ded5919
Compare
jrainville
left a comment
jrainville
left a comment
There was a problem hiding this comment.
Huge work. Very nice!
My only concern is with the performance of Desktop apps. Most Desktop apps don't care that much about being in the background or foreground.
There was a problem hiding this comment.
The archive cannot work on mobile, so I'm not sure if those changes are needed. I know we already have some code where we speed up the archive in the background since the app is not in use. Maybe this code comes in conflict?
There was a problem hiding this comment.
that's true. It's not needed for mobile builds since we're using disable_torrent.
But I'd keep it for completeness. Otherwise we'll need some explicit rules for what's allowed to run in "background" mode.
Could be an overkill at this point.
There was a problem hiding this comment.
Maybe this code comes in conflict?
I don't see any conflict with other mechanisms.
| datasync.SetPausedBackground(paused) | ||
| if m.pushNotificationClient != nil { | ||
| if paused { | ||
| m.pushNotificationClient.Offline() |
There was a problem hiding this comment.
Maybe I'm misunderstanding, but shouldn't we want the push notifications to keep working as usual?
There was a problem hiding this comment.
This is communicating with the push notifications server to send push notifications to other devices or to update config options. We shouldn't need it while the app is in the background.
It's not needed at all to receive push notifications
There was a problem hiding this comment.
A lot of those are only run on Desktop. I think on Desktop, it's fine to do most of these operations in the background.
I wonder if we should make toBackground smarter, where it check whether we are charging?
There was a problem hiding this comment.
I wonder if we should make toBackground smarter, where it check whether we are charging?
We can do that, yes! Although not sure what would be the benefit.
server/server.go
Outdated
| // Retry a few times to preserve stable URL reuse semantics. | ||
| if s.config != nil && s.config.AddrPort.Port() == 0 && s.cachedPort != 0 && errors.Is(err, syscall.EADDRINUSE) { | ||
| for i := 0; i < 10; i++ { | ||
| time.Sleep(20 * time.Millisecond) |
There was a problem hiding this comment.
Is this function called syncly? If so, we shouldn't use sleep or we should put it in a routine
There was a problem hiding this comment.
Actually I have a better way of dealing with this without "retry until it works".
We can use SO_REUSEADDR to bind immediately https://man7.org/linux/man-pages/man7/socket.7.html.
Will update
There was a problem hiding this comment.
Updated. I'll need someone to test windows though. It seems the API is a little bit different there.
That's true. And desktop apps don't need to use the |
Introduce a lifecycle-based pause/resume mechanism to reduce non-essential work when the app goes to background, improving battery and resource usage. - Add pkg/messaging/lifecycle for SubscribePausedBackground/SetPausedBackground - Add backend pause/play (pauseLocked/resumeLocked) driven by AppStateChange - StatusNode.PauseBackground/ResumeForeground coordinate services via backgroundLifecycle interface - Messenger.ToForeground/ToBackground broadcast to lifecycle, datasync, and push notification client - Pause tickers/loops in: centralizedmetrics, ipfs, rpc limiter, ntp, backup, transport, envelopes monitor, connector, wallet, newsfeed, ext - Simplify mobile AppStateChange (remove AppStateChangeV2, single API) - RPC client: clean up limiters on Stop
BaseServer now receives a pointer since it owns a mutex. + adding more tests for the lifecycle flows
- remove log file - Use `syscall.SO_REUSEADDR` for media server to allow quick rebind on the same port
alexjba
force-pushed
the
feat/reduce-app-work
branch
from
4821ba4 to
a1ecaed
Compare
| defer common.LogOnPanic() | ||
| ticker := time.NewTicker(time.Second / maxRequestsPerSecond) | ||
| defer ticker.Stop() | ||
| sub := messaginglifecycle.SubscribePausedBackground() |
There was a problem hiding this comment.
These blocks looks identical. Maybe extract it to an utility function ?
func RunPausableTicker(ticker *time.Ticker, quit <-chan struct{}, action func()) {
sub := SubscribePausedBackground()
defer sub.Unsubscribe()
paused := <-sub.C()
var tickerC <-chan time.Time
if !paused {
tickerC = ticker.C
}
for {
select {
case pausedState, ok := <-sub.C():
if !ok { return }
paused = pausedState
if paused { tickerC = nil } else { tickerC = ticker.C }
case <-tickerC:
action()
case <-quit:
return
}
}
}
| return nil | ||
| } | ||
|
|
||
| func (s *Service) PauseBackground() error { |
There was a problem hiding this comment.
maybe extract common lifecycle flow to a mixin?
package common
import "sync"
type LifecycleMixin struct {
mu sync.Mutex
started bool
paused bool
}
func (m *LifecycleMixin) PauseBackground(onPause func() error) error {
m.mu.Lock()
defer m.mu.Unlock()
if !m.started || m.paused {
return nil
}
if err := onPause(); err != nil {
return err
}
m.paused = true
return nil
}
func (m *LifecycleMixin) ResumeForeground(onResume func() error) error {
m.mu.Lock()
defer m.mu.Unlock()
if !m.started || !m.paused {
return nil
}
if err := onResume(); err != nil {
return err
}
m.paused = false
return nil
}
func (m *LifecycleMixin) MarkStarted() { m.mu.Lock(); m.started = true; m.paused = false; m.mu.Unlock() }
func (m *LifecycleMixin) MarkStopped() { m.mu.Lock(); m.started = false; m.paused = false; m.mu.Unlock() }
func (m *LifecycleMixin) IsPaused() bool { m.mu.Lock(); defer m.mu.Unlock(); return m.paused }
|
It would be great to hear what @igor-sirotin thinks. Just to confirm the order of starting and stopping services |
There was a problem hiding this comment.
There is a pkg/pubsub that I think can be used for this
| const ( | ||
| AppLifecycleStopped AppLifecycleState = "stopped" | ||
| AppLifecycleRunning AppLifecycleState = "running" | ||
| AppLifecyclePausedBackground AppLifecycleState = "paused_background" | ||
| AppLifecycleResumingForeground AppLifecycleState = "resuming_foreground" | ||
| ) |
There was a problem hiding this comment.
Last time we were looking at status-go API, we were planning to remove any "app state" notion from the backend — endpoints like AppStateChange and ConnectionChange.
IMO, backend should be agnostic of app's state. Instead, it can have specific behaviour-changing actions, e.g. pauseService(serviceName) or resetConnection. It is application-level details that there is some "background mode", "battery-saving mode" or "expensive connection".
Having such stuff in backend is quite attractive, but it messes the frontend-backend split.
This was created during the "let's implement as much as we can in backend" paradigm, when we had 2 clients. But there should be a concrete border-line of what goes where. E.g. for bots a "background mode" makes no sense.
There was a problem hiding this comment.
E.g. for bots a "background mode" makes no sense.
Unless it runs on your phone 😄
I see the point! For a similar reason I've found it difficult to exclude some things that are not necessary for mobile chat - like archive manager. Because we're missing a proper inventory and it's easier to track active services and loops if it's all or nothing.
Ok, so in conclusion the best way forward would be (please correct me if I got it wrong):
-
create some sort of inventory for services and capabilities for each service. Basically status-go would expose a list of capabilities and the client would then be able to do a granular control.
-
add APIs to pause (or stop) services
-
add APIs to control some service capabilities.
-
configure the client to do a granular pause while in the background.
There was a problem hiding this comment.
Yup, I think this would be better 👍
| return nil | ||
| } | ||
|
|
||
| func (s *Service) PauseBackground() error { |
There was a problem hiding this comment.
Why not just Pause and Resume?
StatusApp PR status-im/status-app#20202
Iterates status-im/status-app#20227
Introduce a lifecycle-based pause/resume mechanism to reduce non-essential
work when the app goes to background, improving battery and resource usage. This is needed to reduce the battery impact when status-go runs as a background service.
backgroundLifecycle interface
and push notification client
backup, transport, envelopes monitor, connector, wallet, newsfeed, ext
Other changes: