fix(blockstm): wake up suspended executors on cancel #25793
Conversation
to prevent goroutine leaks and hanging when scheduler doesn't complete
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25793 +/- ##
==========================================
- Coverage 70.84% 68.59% -2.25%
==========================================
Files 890 889 -1
Lines 58024 58017 -7
==========================================
- Hits 41106 39797 -1309
- Misses 16918 18220 +1302
... and 38 files with indirect coverage changes 🚀 New features to boost your workflow:
|
blockstm/status.go
Outdated
| s.Lock() | ||
| if s.status == StatusSuspended && s.cond != nil { | ||
| s.cond.Notify() | ||
| s.cond = nil |
There was a problem hiding this comment.
we need to set the status to StatusExecuting so it don't break any invariant, because the executor may still in the process of executing transaction.
There was a problem hiding this comment.
we also need to clear the estimation marks, otherwise, the executor still can't break out of the loop.
blockstm/stm.go
Outdated
| err := ctx.Err() | ||
| // canceled, wake up all suspended executors to prevent hanging | ||
| scheduler.CancelAll() | ||
| canceled := scheduler.CancelAll() |
There was a problem hiding this comment.
If there are executors hangs, the errgroup won't finish in the first place?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| go func() { | ||
| select { | ||
| case <-ctx.Done(): | ||
| canceled := scheduler.CancelAll() | ||
| for _, txn := range canceled { | ||
| mvMemory.ClearEstimates(txn) | ||
| } | ||
| case <-cancelDone: | ||
| } | ||
| }() |
Check notice
Code scanning / CodeQL
Spawning a Go routine
3 participants
Description
to prevent goroutine leaks and hanging when scheduler doesn't complete
Closes: #25789