-
Notifications
You must be signed in to change notification settings - Fork 37
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
Invocation task inactivity timeout(s) #237
Conversation
I tested locally and it works, but needs #235 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this PR @slinkydeveloper. The changes look good to me. +1 for merging after resolving the comments.
src/invoker/src/invocation_task.rs
Outdated
@@ -51,6 +52,9 @@ pub(crate) enum InvocationTaskError { | |||
Network(hyper::Error), | |||
#[error("unexpected join error, looks like hyper panicked: {0}")] | |||
UnexpectedJoinError(#[from] JoinError), | |||
// TODO introduce hint with CodedError to increase timeout config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :-)
src/invoker/src/invocation_task.rs
Outdated
@@ -374,19 +386,35 @@ where | |||
} | |||
} | |||
}, | |||
_ = tokio::time::sleep(self.bidi_stream_inactivity_graceful_close_timer) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see how natural the timeout implementation integrates into the existing code :-)
77831dd
to
8dd603b
Compare
8dd603b
to
7e54f7b
Compare
Fix #222