Queue async monitor update writes synchronously#4671
Conversation
Monitor update persistence relies on KVStore write calls being queued in call order. The incremental async update path deferred that write until executor polling, allowing async scheduling to reorder persistence operations relative to later updates or cleanup. Add a regression test that builds an incremental update future without polling it and verifies that the write has already been queued. Co-Authored-By: HAL 9000 This finding was discovered by Project Loupe
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
The fix is correct and consistent with the No issues found. |
tnull
force-pushed
the
2026-06-async-monitor-update-ordering
branch
from
a3967b5 to
8ac52cf
Compare
|
Updated to line-wrap commit messages. |
| }); | ||
| let write_fut: Pin< | ||
| Box<dyn MaybeSendableFuture<Output = Result<(), io::Error>> + 'static>, | ||
| > = Box::pin(self.kv_store.write( |
There was a problem hiding this comment.
I don't actually think this is a bug, the comment is just wrong. Its not crazy to fix this, however, but we should do it like the code in the else block, not by Box::pining.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
4 participants
Monitor update persistence relies on KVStore write calls being queued in call order. The incremental async update path deferred that write until executor polling, allowing async scheduling to reorder persistence operations relative to later updates or cleanup.
Add a regression test that builds an incremental update future without polling it and verifies that the write has already been queued.
Co-Authored-By: HAL 9000
This finding was discovered by Project Loupe