Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send terminated output when canceling a concurrent input. #2382

Merged
merged 8 commits into from
Oct 30, 2024

Conversation

danielshaar
Copy link
Contributor

@danielshaar danielshaar commented Oct 22, 2024

The worker will ignore the terminated outputs for now. We should wait until at least Thursday 4pm ET before releasing this to ensure all workers have rolled out with the ignoring logic.

Context in WRK-11.

@danielshaar danielshaar marked this pull request as draft October 22, 2024 22:13
@freider
Copy link
Contributor

freider commented Oct 23, 2024

We don't need to send these messages if only one input is running on a container.
Why not? Concurrency=1-containers also need to escalate if they don't respond gracefully to single input cancellations, and I think it would be nice if the worker can always assume getting a FunctionPutOutputs for each input unless something bad happens

@freider
Copy link
Contributor

freider commented Oct 23, 2024

We might also want to add some assertions in relevant container_tests (there are plenty for various different cancellation conditions) that FunctionPutOutputs is sent

@danielshaar danielshaar force-pushed the dshaar/input-cancellation-sends-terminated-output branch from 42fa468 to 6e00066 Compare October 23, 2024 13:58
@danielshaar danielshaar marked this pull request as ready for review October 23, 2024 14:26
@danielshaar danielshaar force-pushed the dshaar/input-cancellation-sends-terminated-output branch 2 times, most recently from e382543 to eb5d6cb Compare October 23, 2024 18:10
@danielshaar danielshaar force-pushed the dshaar/input-cancellation-sends-terminated-output branch 2 times, most recently from 8fab2e4 to b41b8d9 Compare October 29, 2024 12:41
Copy link
Contributor

@freider freider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@danielshaar danielshaar force-pushed the dshaar/input-cancellation-sends-terminated-output branch from 846f669 to c2e1f07 Compare October 30, 2024 14:46
@danielshaar danielshaar merged commit 5f10a8a into main Oct 30, 2024
18 checks passed
@danielshaar danielshaar deleted the dshaar/input-cancellation-sends-terminated-output branch October 30, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants