unref accounts from destroyed forks#19402
unref accounts from destroyed forks#19402jeffwashington wants to merge 2 commits intosolana-labs:v1.6from
Conversation
jeffwashington
changed the title
jeffwashington
force-pushed
the
non-clean-1.6_jwash2
branch
from
b6b3fc4 to
9148c2d
Compare
|
|
||
| for s in &all_removed_slot_storages { | ||
| for (_store_id, store) in s.read().unwrap().iter() { | ||
| if !store.unref_done.swap(true, Ordering::Relaxed) { |
There was a problem hiding this comment.
@sakridge here's my first attempt. All tests pass with the exception of a race condition I'm debugging in:
test_store_scan_consistency_unrooted
There was a problem hiding this comment.
I see. So if we may go through the unref path twice in some cases. This would prevent that by keeping a bool per store to prevent it.
There was a problem hiding this comment.
All runtime tests now pass locally. I wasn't handling AccountStorageEntry::recycle.
| for slot in dead_slots.iter() { | ||
| if let Some(slot_storage) = self.storage.get_slot_stores(*slot) { | ||
| for store in slot_storage.read().unwrap().values() { | ||
| let already_unrefd = store.unref_done.swap(true, Ordering::Relaxed); |
There was a problem hiding this comment.
@sakridge here's the other path I found.
Codecov Report
@@ Coverage Diff @@
## v1.6 #19402 +/- ##
=======================================
Coverage 82.2% 82.3%
=======================================
Files 423 423
Lines 118114 118176 +62
=======================================
+ Hits 97197 97265 +68
+ Misses 20917 20911 -6 |
|
we believe @carllin has a 1.7 solution for this that he will back port to 1.6. I have separated the test out for consideration separately in 1.7 and master. |
|
Here is the proposed backport: #19414. It was a bit of a messy merge as there are quite a few fixes on 1.7 that had not been backported to 1.6 |
3 participants
Problem
@sakridge 's original pr:
#19398
Summary of Changes
Fixes #