Skip to content

Commit 0e465fe

Browse files
alexjbaclaude
andcommitted
fix(storenode): community fetches cap at 3 pages and skip empty-page processing (#21470-hf)
Run-4 device evidence (all prior #21470-hf fixes in): dead curated community ids — whose descriptions are never on the store node — each cost a ~minutes-long 30-page run finalizing with fetchedEnvelopesCount=0 at cap-trip, re-issued by client polling ~every 20s. The per-id backoff never suppresses because each RUN outlasts its early backoff windows. The CPU engine: shouldFetchNextPage force-runs Messenger.ProcessAllMessages on EVERY page, including empty ones, and ProcessAllMessages walks the messenger's whole pending backlog. During the legitimate channel backfill that means re-walking a fat queue 30x per dead id, continuously (GC 67%, service 320-370%). Two changes, both in protocol/messenger_store_node_request_manager*.go: 1. Split the page cap by request type. Community fetches now build from a dedicated community config capped at 3 pages (maxCommunityStoreNodeRequestPageCount) via buildCommunityStoreNodeRequestConfig; contact fetches keep the generic 30-page default (buildStoreNodeRequestConfig). Store queries page newest-first (verified in go-waku history.go: PaginationForward unset => proto3 default false => backward) and community descriptions are republished periodically, so a live community's description appears within the first pages; with the description-seen gate (6cd6ced) a hit stops the request anyway, so deeper paging can only ever chew empty/irrelevant history. The shared default is untouched — only the community construction site is tightened, and caller options can still override. 2. Skip the forced per-page ProcessAllMessages when the just-fetched page carried zero envelopes (shouldProcessStoreNodePage). The call only exists to flush THIS request's envelopes into the DB before the GetByID check; with zero envelopes there is nothing of ours to flush, so the backlog re-walk is pure waste. The DB check still runs, so a description delivered by a parallel channel-sync page is not missed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit cae6698) (cherry picked from commit 0b2ee62d39aad5c0aa6134825365a9417723f1b7)
1 parent 59ab161 commit 0e465fe

3 files changed

Lines changed: 124 additions & 3 deletions

protocol/messenger_store_node_request_manager.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func NewStoreNodeRequestManager(m *Messenger) *StoreNodeRequestManager {
7575
// Automatically waits for an available store node.
7676
// When a `nil` community and `nil` error is returned, that means the community wasn't found at the store node.
7777
func (m *StoreNodeRequestManager) FetchCommunity(ctx context.Context, communityID string, opts []StoreNodeRequestOption) (*communities.Community, StoreNodeRequestStats, error) {
78-
cfg := buildStoreNodeRequestConfig(opts)
78+
cfg := buildCommunityStoreNodeRequestConfig(opts)
7979

8080
m.logger.Info("requesting community from store node",
8181
zap.String("community", communityID),
@@ -381,7 +381,21 @@ func (r *storeNodeRequest) shouldFetchNextPageUncapped(envelopesCount int) (bool
381381
// Force all received envelopes to be processed. We keep the resulting
382382
// response so a community request can tell whether this page actually
383383
// carried a description for the target community (issue #21470-hf).
384-
response := r.manager.messenger.ProcessAllMessages()
384+
//
385+
// Skip the walk entirely for an empty page: ProcessAllMessages traverses the
386+
// messenger's whole pending backlog, but its only job here is to flush THIS
387+
// page's envelopes into the DB before the GetByID check. With zero envelopes
388+
// there is nothing of ours to flush, and during a channel backfill re-walking
389+
// that fat queue on every empty page — to the cap, per dead id — is the CPU/GC
390+
// storm behind the device overheat (issue #21470-hf). The DB check below still
391+
// runs, so a description delivered by a parallel channel-sync page is not
392+
// missed; only the redundant processing is dropped.
393+
var response *MessengerResponse
394+
if shouldProcessStoreNodePage(envelopesCount) {
395+
response = r.manager.messenger.ProcessAllMessages()
396+
} else {
397+
logger.Debug("skipping ProcessAllMessages for empty store node page")
398+
}
385399

386400
// Try to get community from database
387401
switch r.requestID.RequestType {

protocol/messenger_store_node_request_manager_config.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ package protocol
99
// full 31-day store-node window and burns CPU/battery on the device.
1010
const maxStoreNodeRequestPageCount = 30
1111

12+
// maxCommunityStoreNodeRequestPageCount caps community fetches far more tightly
13+
// than the generic contact cap (issue #21470-hf). Store queries page newest-first
14+
// (verified in go-waku history.go: PaginationForward unset => proto3 default false
15+
// => backward) and community descriptions are republished periodically, so a live
16+
// community's description lands within the first pages; the description-seen gate
17+
// (6cd6ced1a) then stops the request on that hit. Deeper paging can therefore only
18+
// ever chew empty/irrelevant history — exactly the device failure mode: dead
19+
// curated ids each cost a ~minutes-long 30-page run finalizing with
20+
// fetchedEnvelopesCount=0. 3 pages keeps a healthy fetch (1-2 pages) untouched
21+
// while collapsing a dead-id run to a fraction of its former cost. Applied only at
22+
// the community construction site; the contact/default cap stays at 30.
23+
const maxCommunityStoreNodeRequestPageCount = 3
24+
1225
type StoreNodeRequestConfig struct {
1326
WaitForResponse bool
1427
StopWhenDataFound bool
@@ -89,6 +102,19 @@ func (c StoreNodeRequestConfig) gateNextPageByDescriptionSeen(fetchNext, descrip
89102
return fetchNext, false
90103
}
91104

105+
// shouldProcessStoreNodePage reports whether the forced per-page
106+
// Messenger.ProcessAllMessages should run for a page that carried envelopesCount
107+
// envelopes (issue #21470-hf). ProcessAllMessages walks the messenger's whole
108+
// pending backlog; the store-node pager only forces it to flush THIS request's
109+
// just-fetched envelopes into the DB before the GetByID check. A page with zero
110+
// envelopes has nothing of ours to flush, so processing it only re-walks the
111+
// (possibly fat) backfill queue for no gain — the CPU/GC amplification observed on
112+
// device. Skip it; the subsequent DB check still runs so a description delivered
113+
// by a parallel channel-sync page is not missed.
114+
func shouldProcessStoreNodePage(envelopesCount int) bool {
115+
return envelopesCount > 0
116+
}
117+
92118
type StoreNodeRequestOption func(*StoreNodeRequestConfig)
93119

94120
func defaultStoreNodeRequestConfig() StoreNodeRequestConfig {
@@ -101,16 +127,36 @@ func defaultStoreNodeRequestConfig() StoreNodeRequestConfig {
101127
}
102128
}
103129

104-
func buildStoreNodeRequestConfig(opts []StoreNodeRequestOption) StoreNodeRequestConfig {
130+
// defaultCommunityStoreNodeRequestConfig is the base config for community fetches.
131+
// It matches the shared default in every field except the page cap, which is
132+
// tightened to maxCommunityStoreNodeRequestPageCount (issue #21470-hf).
133+
func defaultCommunityStoreNodeRequestConfig() StoreNodeRequestConfig {
105134
cfg := defaultStoreNodeRequestConfig()
135+
cfg.MaxPageCount = maxCommunityStoreNodeRequestPageCount
136+
return cfg
137+
}
106138

139+
func applyStoreNodeRequestOptions(cfg StoreNodeRequestConfig, opts []StoreNodeRequestOption) StoreNodeRequestConfig {
107140
for _, opt := range opts {
108141
opt(&cfg)
109142
}
110143

111144
return cfg
112145
}
113146

147+
// buildStoreNodeRequestConfig builds the config for a contact fetch (the generic
148+
// default, 30-page cap). Caller options are folded on top.
149+
func buildStoreNodeRequestConfig(opts []StoreNodeRequestOption) StoreNodeRequestConfig {
150+
return applyStoreNodeRequestOptions(defaultStoreNodeRequestConfig(), opts)
151+
}
152+
153+
// buildCommunityStoreNodeRequestConfig builds the config for a community fetch,
154+
// starting from the tighter community default (3-page cap). Caller options are
155+
// still folded on top and can override the cap (issue #21470-hf).
156+
func buildCommunityStoreNodeRequestConfig(opts []StoreNodeRequestOption) StoreNodeRequestConfig {
157+
return applyStoreNodeRequestOptions(defaultCommunityStoreNodeRequestConfig(), opts)
158+
}
159+
114160
func WithWaitForResponseOption(waitForResponse bool) StoreNodeRequestOption {
115161
return func(c *StoreNodeRequestConfig) {
116162
c.WaitForResponse = waitForResponse

protocol/messenger_store_node_request_manager_config_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,67 @@ func TestStoreNodeRequestPageCap(t *testing.T) {
6464
})
6565
}
6666

67+
// TestCommunityStoreNodeRequestPageCap verifies that community fetches get a much
68+
// tighter page cap than contact fetches (issue #21470-hf). Store queries page
69+
// newest-first (go-waku history.go: PaginationForward unset = backward) and
70+
// community descriptions are republished periodically, so a live community's
71+
// description appears within the first few pages; with the description-seen gate
72+
// (6cd6ced1a) a hit stops the request anyway, so deep paging can only ever chew
73+
// empty/irrelevant history. The device evidence: dead curated ids each cost a
74+
// ~minutes-long 30-page run with fetchedEnvelopesCount=0 at cap-trip. Contact
75+
// fetches keep the generous 30-page cap; only the community construction site is
76+
// tightened, never the shared default.
77+
func TestCommunityStoreNodeRequestPageCap(t *testing.T) {
78+
t.Run("community construction caps at 3 pages", func(t *testing.T) {
79+
require.Equal(t, 3, maxCommunityStoreNodeRequestPageCount)
80+
cfg := buildCommunityStoreNodeRequestConfig(nil)
81+
require.Equal(t, maxCommunityStoreNodeRequestPageCount, cfg.MaxPageCount)
82+
})
83+
84+
t.Run("contact construction keeps the generous 30-page cap", func(t *testing.T) {
85+
require.Equal(t, 30, maxStoreNodeRequestPageCount)
86+
cfg := buildStoreNodeRequestConfig(nil)
87+
require.Equal(t, maxStoreNodeRequestPageCount, cfg.MaxPageCount)
88+
})
89+
90+
t.Run("community construction is tighter than contact construction", func(t *testing.T) {
91+
community := buildCommunityStoreNodeRequestConfig(nil)
92+
contact := buildStoreNodeRequestConfig(nil)
93+
require.Less(t, community.MaxPageCount, contact.MaxPageCount)
94+
})
95+
96+
t.Run("community construction inherits the shared non-cap defaults", func(t *testing.T) {
97+
cfg := buildCommunityStoreNodeRequestConfig(nil)
98+
base := defaultStoreNodeRequestConfig()
99+
require.Equal(t, base.WaitForResponse, cfg.WaitForResponse)
100+
require.Equal(t, base.StopWhenDataFound, cfg.StopWhenDataFound)
101+
require.Equal(t, base.InitialPageSize, cfg.InitialPageSize)
102+
require.Equal(t, base.FurtherPageSize, cfg.FurtherPageSize)
103+
})
104+
105+
t.Run("caller options still override the community cap", func(t *testing.T) {
106+
cfg := buildCommunityStoreNodeRequestConfig([]StoreNodeRequestOption{WithMaxPageCount(7)})
107+
require.Equal(t, 7, cfg.MaxPageCount)
108+
})
109+
}
110+
111+
// TestShouldProcessStoreNodePage verifies the decision that skips the forced
112+
// per-page Messenger.ProcessAllMessages when the just-fetched page carried zero
113+
// envelopes (issue #21470-hf). That call exists solely to flush THIS request's
114+
// envelopes into the DB before the GetByID check; with zero envelopes there is
115+
// nothing of ours to flush, yet ProcessAllMessages still walks the messenger's
116+
// whole pending backlog. During the legitimate channel backfill that means
117+
// re-walking a fat queue on every empty page — up to the page cap, continuously,
118+
// for each dead curated id — the CPU/GC storm seen on device (GC 67%, service
119+
// 320-370%). Skipping the walk on empty pages removes that amplification; the DB
120+
// check still runs (a parallel channel-sync page may have delivered the
121+
// description).
122+
func TestShouldProcessStoreNodePage(t *testing.T) {
123+
require.False(t, shouldProcessStoreNodePage(0), "empty page has nothing of ours to flush")
124+
require.True(t, shouldProcessStoreNodePage(1), "a page with envelopes must be processed")
125+
require.True(t, shouldProcessStoreNodePage(50), "a full page must be processed")
126+
}
127+
67128
// TestGateNextPageByValidationQueue verifies the decision that stops store-node
68129
// pagination for a community request once the community's description is already
69130
// in hand and only blocked on owner validation (issue #21470-hf). For a

0 commit comments

Comments
 (0)