Skip to content

Make the source shutdown instantaneous #1771

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 14, 2021
Merged

Make the source shutdown instantaneous #1771

merged 3 commits into from
Jul 14, 2021

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Jul 13, 2021 •
edited
Loading

📔 Description

This changes the source actor not to do busy work in the processing stage of its stream source so it stays responsive.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Give it a spin locally with various configurations

@dominiklohmann dominiklohmann added the bug Incorrect behavior label Jul 13, 2021
@dominiklohmann dominiklohmann requested a review from lava July 13, 2021 04:54
@dominiklohmann dominiklohmann mentioned this pull request Jul 13, 2021
Closed
3 tasks
@dominiklohmann dominiklohmann force-pushed the topic/dummy-transformer-sink branch 2 times, most recently from 7bb5ea0 to 788f062 Compare July 13, 2021 08:20
@dominiklohmann
Make the source shutdown instantaneous
21b7c92 
This changes the source actor not to do busy work in the processing
stage of its stream source so it stays responsive.
@dominiklohmann dominiklohmann force-pushed the topic/dummy-transformer-sink branch from 788f062 to 21b7c92 Compare July 13, 2021 08:49
lava
lava approved these changes Jul 14, 2021
View reviewed changes
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me, and we already extensively discussed the mechanism. I didn't personally test, but the experience from our test bed should be enough imho. So just a few minor comments below.

@dominiklohmann dominiklohmann merged commit 7a29862 into master Jul 14, 2021
@dominiklohmann dominiklohmann deleted the topic/dummy-transformer-sink branch July 14, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLoser JoeLoser JoeLoser left review comments

@lava lava lava approved these changes

Assignees
No one assigned
Labels
bug Incorrect behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dominiklohmann @lava @JoeLoser