Skip to content

Comments

Random backup manager fixes#469

Open
matte1 wants to merge 4 commits intosift-stack:mainfrom
matte1:matte1/backup_manager_fixes
Open

Random backup manager fixes#469
matte1 wants to merge 4 commits intosift-stack:mainfrom
matte1:matte1/backup_manager_fixes

Conversation

@matte1
Copy link
Contributor

@matte1 matte1 commented Feb 8, 2026

Random fixes to the sift stream backup manager.

@tsift
Copy link
Contributor

tsift commented Feb 10, 2026

Thanks for the PR @matte1 we really appreciate it! The changes look good to me, and great comments! I think it just needs a run of cargo fmt and cargo clippy (and resolving any findings from clippy).

For merging, we follow a commit message pattern of [LANG(NATURE) | NATURE]: MSG
For these changes something of the form rust(bug): MSG.

@matte1 matte1 force-pushed the matte1/backup_manager_fixes branch from 0159861 to 6dd2913 Compare February 11, 2026 06:00
error = %e,
"unable to write buffered data during drop, data may be lost"
);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if the write here fails, attempting sync_all as well might help limit what could be lost. But if the write is failing, probably a good chance that sync_all will also fail.

Verification:
Added unit test that demonstrates the issue. Testing this without the
fix causes this unit test to fail.
Verification:
Added a unit test that previously would fail and now passes.
Verification:
Added unit tests showing the that message id of 0 is handled correctly.
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