Issue 100: Vol destroy with outstanding IO#99
Conversation
yamingk
changed the title
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
===========================================
+ Coverage 61.68% 80.49% +18.80%
===========================================
Files 15 17 +2
Lines 462 1102 +640
Branches 35 121 +86
===========================================
+ Hits 285 887 +602
+ Misses 158 153 -5
- Partials 19 62 +43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/lib/homeblks_impl.cpp
Outdated
| } | ||
|
|
||
| void HomeBlocksImpl::start_reaper_thread() { | ||
| auto const nsecs = gc_timer_secs(); |
| superblk< vol_sb_t > sb_; // meta data of the volume | ||
|
|
||
| sisl::atomic_counter< uint64_t > outstanding_reqs_{0}; // number of outstanding requests | ||
| bool destroy_started_{ |
There was a problem hiding this comment.
can we use enum instead vol_state::destroying ?
There was a problem hiding this comment.
it is actually different states, so when we are rebooting, the state will be vol_state::destroying, but destroy has never been kicked off yet after reboot, that is why I introduced this field.
| } | ||
| } | ||
| // Volume Destructor will be called after vol_ptr goes out of scope; | ||
| return folly::Unit(); |
There was a problem hiding this comment.
Is it intentional that the caller cant know the status of the remove_volume ?
There was a problem hiding this comment.
yes, it is intentional since it is async and lazy free of the blocks. Do you see any problem with this approach?
| } | ||
|
|
||
| if (vol_ptr) { | ||
| vol_ptr->state_change(vol_state::DESTROYING); |
There was a problem hiding this comment.
nit: we could also move this and check can_remove inside the vol_ptr->destroy too.
There was a problem hiding this comment.
I thought about this actually, then I think it would be better to have them in different APIs for better contract between the callers and volume layers and decided to keep it this way.
3 participants
Testing:
100 iterations has passed on my local env.