-
Notifications
You must be signed in to change notification settings - Fork 514
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
Allow timeout for async DoFn #5201
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5201 +/- ##
==========================================
- Coverage 63.05% 62.56% -0.50%
==========================================
Files 300 301 +1
Lines 10947 10807 -140
Branches 740 764 +24
==========================================
- Hits 6903 6761 -142
- Misses 4044 4046 +2 ☔ View full report in Codecov by Sentry. |
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.
Is this a speculated fix to the hanging pipelines?
Yes. One user reported that setting the timeout will at least retry the bundle instead of hanging forever. It would be also nice to set the timeout as an option for the GrpcDoFn |
@@ -87,7 +110,7 @@ public void onFailure(Throwable t) { | |||
} | |||
} | |||
}, | |||
MoreExecutors.directExecutor()); |
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.
MoreExecutors.directExecutor() doc says callback may not be executed if the exception terminates the executing thread
In other cases, no code will catch the exception, and it may terminate whichever thread happens to trigger the execution.
I've changed to use another executor that also completes the future when if it refuses to execute the callback. This is the strategy used by guava when transforming futures.
Closing for now in favor of #5209 will reconsider if not solved |
No description provided.