lukecwik commented on a change in pull request #12994:
URL: https://github.com/apache/beam/pull/12994#discussion_r498937353
##########
File path:
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/util/common/worker/MapTaskExecutor.java
##########
@@ -146,13 +165,17 @@ public boolean supportsRestart() {
@Override
public void abort() {
// Signal the read loop to abort on the next record.
- // TODO: Also interrupt the execution thread.
for (Operation op : operations) {
Preconditions.checkState(op instanceof ReadOperation || op instanceof
ReceivingOperation);
if (op instanceof ReadOperation) {
((ReadOperation) op).abortReadLoop();
}
}
+ synchronized (this) {
+ if (currentExecutorThread != null) {
+ currentExecutorThread.interrupt();
Review comment:
Internally within Flume we saw with the C++ worker where user code
didn't handle thread::cancel correctly and were relying on process crash to not
get stuck.
I would generally love for us to be able to interrupt random code
arbitrarily as it would be the best and cleanest way to do this and if we want
to do this we should really have a way for users to opt-out incase it doesn't
work for them. We can watch how many people opt-out and find out why and
possibly make this the default forever or choose to make this the default in
portable execution as that will require a migration.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
