fix: rollback pending state on tempfile failure in prepare_add#2689
fix: rollback pending state on tempfile failure in prepare_add#2689srpatcha wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2689 +/- ##
==========================================
+ Coverage 74.17% 74.34% +0.16%
==========================================
Files 70 70
Lines 39207 39396 +189
==========================================
+ Hits 29083 29288 +205
+ Misses 10124 10108 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
would it be possible to have a unit test to cover this change? thanks |
Per @sylvestre review request on mozilla#2689, add a unit test that exercises the failure path of prepare_add and asserts that pending_size is not incremented when tempfile creation fails. Before the fix, prepare_add updated pending state BEFORE calling tempfile_in(), so any tempfile failure left pending_size > 0 forever, which would later cause make_space() to panic on the next add. The new test forces tempfile failure by removing the cache root after construction, then verifies size() == baseline. Signed-off-by: Srikanth Patchava <srpatcha@users.noreply.github.com>
|
Hi @sylvestre, thank you for the suggestion! Added a regression test in commit
Before the fix, the test would fail with |
|
please run rustfmt on the change |
Per @sylvestre review request on mozilla#2689, add a unit test that exercises the failure path of prepare_add and asserts that pending_size is not incremented when tempfile creation fails. Before the fix, prepare_add updated pending state BEFORE calling tempfile_in(), so any tempfile failure left pending_size > 0 forever, which would later cause make_space() to panic on the next add. The new test forces tempfile failure by removing the cache root after construction, then verifies size() == baseline. Signed-off-by: Srikanth Patchava <srpatcha@users.noreply.github.com>
0a5cfd8 to
7687b70
Compare
|
Hi @sylvestre, ran |
prepare_add updated pending size before tempfile_in(), which never rolled back on failure. This caused make_space() to panic later when encountering negative available space. Moved state update after successful tempfile creation and replaced .expect() with ? operator. Signed-off-by: Srikanth Patchava <spatchava@meta.com>
Per @sylvestre review request on mozilla#2689, add a unit test that exercises the failure path of prepare_add and asserts that pending_size is not incremented when tempfile creation fails. Before the fix, prepare_add updated pending state BEFORE calling tempfile_in(), so any tempfile failure left pending_size > 0 forever, which would later cause make_space() to panic on the next add. The new test forces tempfile failure by removing the cache root after construction, then verifies size() == baseline. Signed-off-by: Srikanth Patchava <srpatcha@users.noreply.github.com>
7687b70 to
3edc72f
Compare
Summary
\prepare_add\ updated pending size before \ empfile_in(), which never rolled back on failure. This caused \make_space()\ to panic later when encountering negative available space.
Moved state update after successful tempfile creation and replaced .expect()\ with ?\ operator.
This is split from #2679 per reviewer feedback to keep changes focused.
Changes
Signed-off-by: Srikanth Patchava spatchava@meta.com