Skip to content

feat: add rate limited publisher#817

Open
wakonig wants to merge 2 commits into
mainfrom
feature/rate_limited_publish
Open

feat: add rate limited publisher#817
wakonig wants to merge 2 commits into
mainfrom
feature/rate_limited_publish

Conversation

@wakonig
Copy link
Copy Markdown
Member

@wakonig wakonig commented Apr 14, 2026

This PR adds rate limiting to ophyd callbacks: A shared pipeline publisher is introduced to coalesce messages during a cooldown window and ensures that the latest message is emitted afterwards. On local tests, this was running with around 300 k msgs / s.

A warning is issued if the update rate per topic is higher than 200 Hz. This should give us better debugging feedback next time we encounter "funny" ECMC problems

@wakonig wakonig force-pushed the feature/rate_limited_publish branch from d16074d to 681a032 Compare April 14, 2026 07:15
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 93.16770% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
..._server/devices/rate_limited_pipeline_publisher.py 94.03% 5 Missing and 4 partials ⚠️
.../bec_server/device_server/devices/devicemanager.py 80.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@wakonig wakonig force-pushed the feature/rate_limited_publish branch 3 times, most recently from fa2a5b5 to eb1489a Compare May 7, 2026 18:33
@wakonig wakonig marked this pull request as ready for review May 7, 2026 18:40
Copilot AI review requested due to automatic review settings May 7, 2026 18:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a shared, rate-limited Redis pipeline publisher for device-server ophyd callbacks to coalesce high-frequency updates per topic and optionally raise a warning when input update rates exceed a threshold.

Changes:

  • Added RateLimitedPipelinePublisher background worker that batches Redis set / set_and_publish operations and coalesces per-topic updates during a cooldown window.
  • Updated DeviceManagerDS ophyd callbacks (limits/readback/config/status/progress) to publish via the rate-limited publisher and added shutdown wiring.
  • Added unit tests for the new publisher and adjusted existing device-manager tests to account for the new publish path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
bec_server/bec_server/device_server/devices/rate_limited_pipeline_publisher.py New coalescing publisher with background flush thread and optional high-update-rate warning alarm.
bec_server/bec_server/device_server/devices/devicemanager.py Routes several ophyd callbacks through the new rate-limited publisher and shuts it down on manager shutdown.
bec_server/tests/tests_device_server/test_rate_limited_pipeline_publisher.py New tests covering coalescing behavior, per-topic isolation, shutdown flush, and warning alarm.
bec_server/tests/tests_device_server/test_device_manager_ds.py Updates progress callback test to mock the new publish mechanism; reorders file-event test block.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bec_server/bec_server/device_server/devices/rate_limited_pipeline_publisher.py Outdated
Comment thread bec_server/bec_server/device_server/devices/devicemanager.py
Comment thread bec_server/tests/tests_device_server/test_device_manager_ds.py
Comment thread bec_server/tests/tests_device_server/test_rate_limited_pipeline_publisher.py Outdated
@wakonig wakonig force-pushed the feature/rate_limited_publish branch 3 times, most recently from d6df135 to 0b8640f Compare May 8, 2026 08:26
@wakonig wakonig force-pushed the feature/rate_limited_publish branch from 0b8640f to 4579c55 Compare May 8, 2026 08:58
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