ore: pend in wait_and_assert_finished instead of yielding once #23588
Conversation
guswynn
force-pushed
the
cherry-pick-for-dan
branch
3 times, most recently
from
bc63dd1 to
4fb6158
Compare
guswynn
changed the title
There was a problem hiding this comment.
Triggered nightly, would appreciate waiting for the result: https://buildkite.com/materialize/nightlies/builds/5446
There was a problem hiding this comment.
Nightly seems good except this failure: https://buildkite.com/materialize/nightlies/builds/5446#018c2275-b361-4315-b7af-0aedaa249180
After ingesting data we had a hang. I'm not sure if it's related to this PR, retriggered.
guswynn
force-pushed
the
cherry-pick-for-dan
branch
from
4fb6158 to
d85832b
Compare
|
rebased, fixed nits, re-triggered nightlies |
| .await | ||
| .into_iter() | ||
| .map(JoinHandle::abort_on_drop) | ||
| .collect(), |
There was a problem hiding this comment.
I defer to @danhhz, but the reason I had the explicit Drop impl was to avoid the reallocation. Could plausibly not matter at all here, but when I wrote the Drop impl I wasn't sure so I erred on the side of caution.
There was a problem hiding this comment.
"The reallocation" here is a second vec? I'd say that's fine here, this gets called 1:1 with registered a reader, which is an expensive operation
|
@benesch i think this is ready for another look |
A singular
yield_nowis not sufficient to yield a task to the tokio runtime if theJoinHandleis being selected or used in things likeFuturesUnordered, so instead, we pend forever. To prevent misuse causing deadlocks, I also altered the return value ofmz_orespawn methods/functions to return a type that uses ownership to prevent users from aborting a task and also callingwait_and_assert_finishedfixes a flake described here: https://materializeinc.slack.com/archives/C01CFKM1QRF/p1701365433621259 (and elsewhere)
Motivation
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.