[CHORE] Cancel tasks spawned on compute runtime #3128
Conversation
CodSpeed Performance ReportMerging #3128 will not alter performanceComparing Summary
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3128 +/- ##
==========================================
- Coverage 79.00% 79.00% -0.01%
==========================================
Files 634 634
Lines 76912 76936 +24
==========================================
+ Hits 60763 60781 +18
- Misses 16149 16155 +6
|
src/common/runtime/src/lib.rs
Outdated
| // Spawn it on a joinset on the runtime, such that if the future gets dropped, the task is cancelled | ||
| let mut joinset = tokio::task::JoinSet::new(); | ||
| joinset.spawn_on(future, self.runtime.handle()); | ||
| async move { joinset.join_next().await.expect("just spawned task") }.boxed() |
There was a problem hiding this comment.
we shouldn't have to box this right? We should avoid the heap allocation if we can here
There was a problem hiding this comment.
I also think it would be cleaner to make a struct that holds the joinset and then impl the future trait on that struct.
Similarly to what influx does for the their Job struct
https://github.com/metrico/influxdb_iox/blob/ab17bbc9efbb8568ea5a95ccb9d4bbddd33fc9ea/executor/src/lib.rs#L88
There was a problem hiding this comment.
yeah good catch no box needed since there's only 1 branch
There was a problem hiding this comment.
Encapsulated the fut in a RuntimeTask struct
Spawns compute tasks on joinsets so that they can be cancelled. --------- Co-authored-by: Colin Ho <[email protected]>
Spawns compute tasks on joinsets so that they can be cancelled.