Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Old statuses are kept unnecessarily #111

Closed
callumforrester opened this issue Apr 5, 2023 · 2 comments
Closed

Old statuses are kept unnecessarily #111

callumforrester opened this issue Apr 5, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@callumforrester
Copy link
Contributor

When producing TaskEvents. The worker includes statuses for completed operations e.g. moving a motor between two previous points in a scan. These are not needed and significantly bloat the size of the event, causing performance issues in client software.

@callumforrester callumforrester added the bug Something isn't working label Apr 5, 2023
@callumforrester callumforrester added this to the Production Readiness milestone Apr 5, 2023
@callumforrester callumforrester self-assigned this Apr 5, 2023
@callumforrester callumforrester changed the title Old status are kept unnecessarily Old statuses are kept unnecessarily Apr 14, 2023
@callumforrester
Copy link
Contributor Author

@keithralphs The bookkeeping is done, apparently wrong, in this class: https://github.com/DiamondLightSource/blueapi/blob/main/src/blueapi/worker/reworker.py#L194

joeshannon added a commit that referenced this issue May 15, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.
joeshannon added a commit that referenced this issue May 16, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.
joeshannon added a commit that referenced this issue May 16, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.
joeshannon added a commit that referenced this issue May 17, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.
joeshannon added a commit that referenced this issue May 18, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.
joeshannon added a commit that referenced this issue May 18, 2023
For some devices there may be an additional status update broadcast
after the on_complete callback. This causes the status to be added
back to the dictionary where it will never be removed.

Track which statuses have been completed and check this set when
updating to ensure that a completed status is never added back to
the snapshot after on_complete is called.

* Add module for test devices

* Add devices module to test environment

* Add test for progress events
@joeshannon
Copy link
Contributor

Fixed with #203

rosesyrett pushed a commit that referenced this issue Nov 10, 2023
)

As well as data documents, blueapi produces events when the status
objects monitored by the run engine are updated. These events are useful
for creating progress bars and similar updates. Unfortunately it seems
very easy to unintentionally make plans/devices produce a very large
number of these updates. The handling of all of these results in log
spam and high CPU usage.

We're seeing this now on I22 and have seen similar problems before (see
#111). I think the easy way to make debugging easier is to make the
status update handling optional and easy to turn off via config. To that
end...

Changes:
- Add config option to disable status events
- Make the worker only hook into the run engine if this option is marked
as true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants