Skip to content

overlay: cap twostep FEC broadcast cache size#2332

Open
Flamki wants to merge 5 commits into
ton-blockchain:testnetfrom
Flamki:audit-r2
Open

overlay: cap twostep FEC broadcast cache size#2332
Flamki wants to merge 5 commits into
ton-blockchain:testnetfrom
Flamki:audit-r2

Conversation

@Flamki

@Flamki Flamki commented Apr 25, 2026

Copy link
Copy Markdown

Summary

  • fix unbounded growth of BroadcastsTwostep::broadcasts_ by enforcing OverlayImpl::max_fec_bcasts()
  • apply eviction both during periodic GC and immediately before inserting a new twostep-FEC decoder state
  • keep eviction behavior consistent with the existing LRU + register_delivered_broadcast flow

Fixes #2223

Details

  • add a small helper to evict the oldest incomplete twostep broadcast state and mark it as delivered
  • call gc(overlay) and then enforce the hard size cap before allocating/storing a new decoder entry
  • keep existing 60s age-based cleanup and add size-based cleanup in BroadcastsTwostep::gc

Validation

  • built the changed translation unit:
    • ninja -C build-win-r2-seq-localssl overlay/CMakeFiles/overlay.dir/broadcast-twostep.cpp.obj -j 1
  • attempted full test-overlay build, but this environment hits unrelated MinGW OOM in heavy units (adnl/*, tonops.cpp) before link

@DanShaders

Copy link
Copy Markdown
Collaborator

attempted full test-overlay build, but this environment hits unrelated MinGW OOM in heavy units (adnl/*, tonops.cpp) before link

Tell Claude that it should actually test changes.

I think this is good but please add tests for this GC akin to DanShaders@27be3a7.

@Flamki

Flamki commented Apr 28, 2026

Copy link
Copy Markdown
Author

Addressed.

  • Added tests for twostep FEC cache-cap GC (plus required overlay/adnl test harness files).
  • Resolved overlay/broadcast-twostep.cpp conflict by keeping both:
    • cache-cap eviction logic
    • upstream limiter (try_register_broadcast) logic
  • Diff is clean: +413 / -10, no conflict markers, no garbled/non-ASCII chars.
  • Built successfully: ninja -C build-win-r2-seq-localssl test-overlay-manager -j 1

Branch head: 717cf875

@DanShaders DanShaders self-requested a review April 29, 2026 00:48
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.

2 participants