Skip to content

fix(l2ps): bound the executed backlog and stop reading it whole every hash cycle - #1063

Open
Shitikyan wants to merge 20 commits into
stabilisationfrom
fix/l2ps-memory-growth
Open

Shitikyan wants to merge 20 commits into
stabilisationfrom
fix/l2ps-memory-growth

Conversation

@Shitikyan

Copy link
Copy Markdown
Contributor

Follow-up to the partner report of a node consuming all 512 GB of its host under L2PS messaging load (only a database wipe recovered it). No live-node profile was possible here, so this is code reading plus measurement against a real Postgres with seeded l2ps_mempool rows.

Mechanism found. The aggregator drains at most 10 rows per batch and one batch per subnet per 10 s cycle (about 1 message/s), while ingest is unbounded. Messaging transactions carry no gcr_edits, so no proof is ever built, and the only code that deletes batched rows runs when a proof is applied: every message left a permanent row with its full encrypted payload. The hash service then loaded the whole EXECUTED backlog, payloads included, twice every 5 s, and a restart reloaded it on the first tick. The container runs under bun, which has no heap cap.

Measured RSS for one hash-service tick (3 KB rows): 200k rows 2.22 GB before, 197 MB after; 1M rows about 10 GB projected before, 211 MB and 1.0 s after. The digest value is unchanged (checked against the old construction on 12,345 and 10,000 seeded rows, several page sizes, an empty subnet and the block filter).

Changes (16 commits, each behavioural change with a test that fails without it)

  • Hash cycle: digest and count come from one REPEATABLE READ transaction over a cursor ordered by hash COLLATE "C", with a running SHA-256 over pages of 5000; on error the subnet is skipped for the cycle instead of publishing an error stand-in.
  • Every insert path (handleL2PS for RPC, omni and manageExecution; peer sync) refuses a hash that is not 64 lowercase hex characters, which is the invariant the ordering relies on.
  • Backpressure: L2PS_MEMPOOL_MAX_EXECUTED_PER_SUBNET (default 50000, 0 disables). Past the cap a subnet's new transactions get 429 L2PS_BACKLOG_FULL with retryAfterMs before any decrypt; the messaging relay answers a retry-later frame and stores nothing. The count is cached 3 s per subnet, a slot is claimed only after decrypt, verification and the duplicate check, the last known count is kept on a failed refresh, and warnings fire once at 50% and at the cap (rate-limited). Peer sync is not gated (synced rows are not EXECUTED).
  • Drain: L2PS_AGGREGATOR_MAX_BATCHES_PER_CYCLE (default 5), fetched per subnet so one busy subnet cannot starve the others.
  • Cleanup: the aggregation cycle deletes BATCHED rows that carry no edits once older than the cleanup age, but only when the permanent history row holds the ciphertext; legacy rows without it are kept and counted in a log line.
  • Info RPC and omni batch-status use COUNT; the omni sync opcode serves pages of at most 1000 with a (timestamp, hash) cursor and hasMore (previously a peer sending no limit received every row). The history fallback looks up only the rows that need it. The messaging quota map drops zero entries.
  • Both variables are documented in .env.example and INSTALL.md.

Ruled out as the cause: the in-memory relay maps (flat at 500k messages), the signalling per-peer state, the register-proof set (time-bounded).

Still open: BATCHED rows with edits whose proof creation failed, and FAILED rows, are never swept; the messaging server has no connection cap or per-socket rate limit; the SDK's messaging client drops the code/retryAfterMs of the retry-later frame (sdks follow-up); a heap profile of a live node under real messaging load is still worth taking.

Tests: all touched files green one at a time; the 11 opt-in Postgres tests pass against postgres:16-alpine; tsc --noEmit unchanged at 107.

Each cycle loaded every executed row twice, payload and edits included, once
for the digest and once to take a length. Measured against Postgres, 200k rows
of about 3 KB each peaked at 2.2 GB resident in the Bun runtime the container
runs, and the cost scales linearly with the backlog. The digest now selects
only the hash column and the size comes from a COUNT query.
The mempool info RPC and the batch-status opcode loaded every executed row to
report a length, and the sync opcode loaded every row of the subnet and sliced
afterwards, with no limit unless the peer sent one. All three are open to any
peer. The first two now use a COUNT, and the sync opcode serves one page of
identifying columns, capped at 1000 entries.
A batched row is deleted when the proof built from its GCR edits is applied at
consensus, and the status that the cleanup looks for is never set anywhere. A
message carries no edits, so its batch produces no proof and the row stayed in
the queue for good: one row, with its full encrypted payload, per message.
The aggregation cycle now also deletes batched rows without edits once they
are older than the cleanup age. Rows with edits are left for their proof.
The per-sender offline counter kept a zero entry after a failed submit or a
delivery, so every distinct key that ever queued a message stayed in the map
for the life of the process. Keys cost nothing to mint, so the map grew with
the number of senders ever seen.
…whole queue

A page of history containing a row stored before ciphertext was kept made the
handler load every executed, batched and confirmed row of the subnet to find
the few it needed. It now asks for exactly the original hashes of the rows
that lack a payload.
Keeps the type-check error count at the base's: jest.mock is not on bun's
jest typings.
… full

The aggregator drains a bounded number of batches per cycle, so ingest above
that rate grew the executed queue without end, and each hash cycle reads it in
full. L2PS_MEMPOOL_MAX_EXECUTED_PER_SUBNET (default 50000, 0 disables) now caps
it per subnet. At the cap the RPC answers 429 with L2PS_BACKLOG_FULL and the
messaging relay answers an L2PS_BACKLOG_FULL error carrying retryAfterMs, before
anything is stored. The count is cached for three seconds and bumped on each
admission, a warning is logged on crossing half the cap and an error, at most
once a minute per subnet, at the cap.
A cycle built one batch of at most ten transactions per subnet, so the queue
drained at ten transactions per interval however many were waiting.
L2PS_AGGREGATOR_MAX_BATCHES_PER_CYCLE (default 5) lets a cycle build that many
batches per subnet, still ten transactions each. A failed submission ends that
subnet's cycle.
The digest loaded every executed hash into one array, sorted it and joined it
into a single string, so memory still grew with the backlog after the payloads
were left out. The digest is now fed to a running SHA-256 a page at a time from
one sorted cursor inside a single repeatable-read transaction, so the count it
leads with and the hashes agree. The value is unchanged: it is the same input
string in byte order, checked against the previous construction on seeded rows,
across page boundaries and with a block filter. At 1M executed rows the
process peaked at about 210 MB resident, against 1.2 GB before.
The outer transaction hash is the queue's primary key and no path recomputes
it, so a subnet-key holder could store any text there. The consolidated digest
was defined over the JavaScript string sort and is now computed in the
database's byte order; the two agree on hex strings and can disagree on others,
for example supplementary-plane characters. addTransaction, the RPC and omni
submit paths and the sync inserts now refuse or drop any hash that is not 64
lower-case hex characters. No constraint is added, since existing rows may not
satisfy it.
Rows written before the history table stored the ciphertext have no payload
there, and the history endpoint recovers theirs from the batched queue row. The
sweep for batched rows without edits now deletes a row only when its history
entry exists with a stored payload, and logs how many legacy rows it kept.
The summary query caught its own errors and returned zero, so the guard's own
error handling never ran and a failed count reopened a subnet that was at its
cap. The query now throws, and the guard keeps the last count it read when a
refresh fails, passing messages only when it has never had a count.
…eued

The slot was taken before the transaction was decrypted, verified or checked
for repeats, so malformed or replayed requests could use up a subnet's room.
The guard now separates a cheap check from a claim, and the submit handler
claims after the duplicate check. The 429 it returns carries the code and the
retry time only, since the RPC and omni paths are open to any caller and the
queue size is not theirs to see.
…hasMore

A cursor on the timestamp alone skips rows that share a millisecond with the
last row of a page. The listing now orders by (timestamp, hash), accepts the
hash with the cursor, and says whether a further row exists.
…rom the digest's snapshot

A read error was answered with a fixed stand-in hash that the hash service then
relayed to validators as the subnet's commitment, and the transaction count came
from a separate query that could disagree with the digest. The mempool now
returns the digest and its count from one repeatable-read transaction and throws
on failure; the hash service skips the subnet for that cycle.
One shared window of the oldest executed rows meant a subnet with a deep
backlog filled it and every other subnet waited. The cycle now lists the
subnets with executed rows and fetches each one's batch window separately.
…cument the uncapped digest

The earlier case claimed to hit a page multiple but seeded 9,900 executed rows.
It now seeds exactly 10,000 and checks the count. The page-size constant moves
above the class comment, and INSTALL.md and .env.example say that with the cap
at 0 the digest sort is unbounded and runs without the primary-key index.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d750feaa-8451-4a3f-8103-84913e541ed5
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The latest changes appear safe to merge, with no new actionable issue found.

Summary

This PR bounds the L2PS queue and avoids loading encrypted payloads during each hash cycle.

  • Hash cycles stream ordered transaction hashes in bounded pages.
  • Full subnets turn away new transactions until the aggregator catches up.
  • The aggregator drains more batches per subnet and removes old no-edit rows.
  • Peer sync and queue status requests read only the rows they need.
  • The messaging service drops sender counts once their offline queue is empty.

The earlier fixes preserve unfinished claims, release failed attempts, reject unusably small caps, and keep completed rows counted. Shitikyan explicitly accepted the remaining brief double-count window as conservative refusal until the next refresh. No new actionable issue was found. Tests were read but not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[New transaction or message] --> B{Subnet has room?}
    B -->|No| C[Return retry-later response]
    B -->|Yes| D[Check request and claim a slot]
    D --> E{Execution succeeds?}
    E -->|No| F[Release claim]
    E -->|Yes| G[Write executed status and settle claim]
    G --> H[Record history]
    G --> I[Read hashes in bounded pages]
    G --> J[Build batches per subnet]
    J --> K[Remove old rows without edits when history holds ciphertext]
Loading

Reviews (4) · Last reviewed commit: "fix(l2ps): settle the backlog slot right..." · Reviewed by Greptile

Comment thread src/libs/l2ps/executedBacklogGuard.ts Outdated
Comment thread src/features/l2ps-messaging/L2PSMessagingService.ts Outdated
Comment thread src/config/loader.ts
…shes, reject a cap below the batch minimum

The executed-backlog guard now tracks claims separately from the database
count, so a refresh no longer forgets slots still in flight and a failed
attempt gives its slot back. The messaging relay claims the subnet slot only
after the sender's offline quota, so a quota rejection never consumes room.

A positive backlog cap below the minimum batch size is rejected at config
load, since the subnet would fill up before its first batch could form. The
guard counts executed rows with a plain indexed count instead of a summary
with MIN/MAX. Shared test mocks replace the duplicated module-mock blocks.
Comment thread src/libs/l2ps/executedBacklogGuard.ts
Comment thread src/libs/l2ps/executedBacklogGuard.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant