Skip to content

fix(memory): make recorder lossless under load - #3558

Open
TomCC7 wants to merge 58 commits into
mainfrom
cc/perf/recorder-perf
Open

fix(memory): make recorder lossless under load#3558
TomCC7 wants to merge 58 commits into
mainfrom
cc/perf/recorder-perf

Conversation

@TomCC7

@TomCC7 TomCC7 commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

Fix Recorder data loss under sustained sensor load and temporary encoder or SQLite stalls. Every message accepted by Recorder is persisted in callback-arrival order, or recording fails visibly.

Approach

  • Add an opt-in bounded queue to pSHMTransport; its default remains the existing latest-value transport.
  • Bypass Recorder's latest-value dispatcher and feed one bounded FIFO worker, which serially calls the existing Stream.append() path.
  • Stop subscriptions, wait for in-flight callbacks to enqueue, then drain accepted work; surface queue overflow, processing/write failure, or drain timeout.

Verification

The standalone recorder benchmark compares the generated source manifest with the completed memory2 database. Its 30-second production profile contains 22,500 observations: RGB 30 Hz, two grayscale streams at 30 Hz, depth 30 Hz, IMU 400 Hz, PointLIO 10 Hz, odometry 200 Hz, and TF 20 Hz.

All three experiments stored 22,500/22,500 observations with no missing, duplicate, corrupt, or reordered payloads:

  • baseline
  • one-second depth encoder stall
  • SQLite writer lock

Each run sustained 64.85 MiB/s offered raw data, 38.88 MiB/s encoded payload, and about 39.07 MiB/s SQLite growth.

  • Targeted Recorder tests: 11 passed
  • Broad memory/pubsub/recorder suite: 511 passed, 1 deselected
  • Ruff, strict mypy, and all hooks applicable to changed files passed

@mintlify

mintlify Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
dimensional 🟢 Ready View Preview Aug 19, 2026, 9:53 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.91954% with 49 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/memory/recording.py 79.61% 12 Missing and 9 partials ⚠️
dimos/memory/module.py 69.69% 16 Missing and 4 partials ⚠️
dimos/memory/test_recording.py 94.24% 6 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main    #3558      +/-   ##
==========================================
+ Coverage   77.10%   77.15%   +0.04%     
==========================================
  Files        1262     1264       +2     
  Lines      120300   120626     +326     
  Branches    10634    11238     +604     
==========================================
+ Hits        92758    93067     +309     
- Misses      24461    24462       +1     
- Partials     3081     3097      +16     
Flag Coverage Δ
OS-ubuntu-24.04-arm 72.05% <82.47%> (+0.06%) ⬆️
OS-ubuntu-latest 73.94% <85.91%> (+0.05%) ⬆️
Py-3.10 73.93% <85.91%> (+0.05%) ⬆️
Py-3.11 73.94% <85.91%> (+0.05%) ⬆️
Py-3.12 73.94% <85.91%> (+0.05%) ⬆️
Py-3.13 73.94% <85.91%> (+0.05%) ⬆️
Py-3.14 73.94% <85.91%> (+0.05%) ⬆️
Py-3.14t 73.94% <85.91%> (+0.05%) ⬆️
SelfHosted-Large 30.01% <20.97%> (-0.03%) ⬇️
SelfHosted-Linux 35.12% <20.97%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/core/transport.py 71.73% <100.00%> (+2.74%) ⬆️
dimos/protocol/pubsub/impl/shmpubsub.py 76.16% <100.00%> (+0.89%) ⬆️
dimos/protocol/pubsub/test_registry.py 97.14% <100.00%> (+1.09%) ⬆️
dimos/memory/test_recording.py 94.24% <94.24%> (ø)
dimos/memory/module.py 61.73% <69.69%> (+5.21%) ⬆️
dimos/memory/recording.py 79.61% <79.61%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TomCC7 TomCC7 changed the title test(memory): characterize recorder fidelity under load fix(memory): make recorder lossless under load Aug 20, 2026
Comment thread dimos/memory/recording.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Comment thread dimos/memory/recording.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Comment thread dimos/memory/recording.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Comment thread dimos/memory/recording.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Comment thread dimos/memory/recording.py Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Comment thread dimos/memory/module.py
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants