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
Interrupt current thread when InterruptException occur #10164
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10164 +/- ##
============================================
+ Coverage 60.52% 60.86% +0.33%
- Complexity 412 447 +35
============================================
Files 1100 1100
Lines 44569 44571 +2
Branches 6492 6493 +1
============================================
+ Hits 26976 27126 +150
+ Misses 14624 14472 -152
- Partials 2969 2973 +4
Continue to review full report at Codecov.
|
if (e.getCause() != null && InterruptedException.class.getName().equals(e.getCause().toString())) { | ||
// don`t catch interrupt exception | ||
throw new RuntimeException(e); | ||
} |
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.
You can choose other fault tolerance strategies, such as cluster="failfast"
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.
It's not a conflict, we need failover strategy at most time, but sometimes due to business needs, I need to interrupt thread pool tasks(which contains dubbo invoke)
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.
Interrupting a thread is not a common scenario, and I don't approve of modifying non-generic logic.
Based on your scenario, I think you may need a custom Cluster implementation.
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.
my code is like this
Future<?> future =RUNNER_EXECUTOR.submit(() -> {
pipelineService.startTask(taskId);
});
//because some logic error, i need find the future and cancel task
future.cancel();
i think it is a normal scenario......
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.
For example, if it is not normal behavior to interrupt the thread, dubbo still needs to initiate a retry. If it is modified according to the above logic, the retry capability will be invalid.
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.
yes, i want disable the retry capability when it is initiative inerrupted by login user
i have to copy failover class and use SPI to override the logic at production which seems ugly
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.
I suggest that you can first create an issue to discuss this requirement, so that more people can see and participate in the discussion.
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.
maybe later, thanks for your patient reply.
would you please add some test cases to verify this |
it is a bit difficult for me to setup the ut environment with mockito, i can only provide basic code consumer:ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor(1, 1, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
String name = "world" + System.currentTimeMillis();
int initialFlag = 0, runningFlag = 1, normalReturn = 2, interruptReturn = 3;
AtomicInteger flag = new AtomicInteger(initialFlag);
Future<?> future = threadPoolExecutor.submit(() -> {
try {
flag.set(runningFlag);
service.sayHello(name);
flag.set(normalReturn);
} catch (Exception e) {
flag.set(interruptReturn);
}
});
Thread.sleep(100);
future.cancel(true);
while (flag.get() == initialFlag || flag.get() == runningFlag) {
Thread.yield();
}
Assertions.assertEquals(flag.get(), interruptReturn); provider:String lastName = "";
@Override
public String sayHello(String name) {
if (!lastName.equals(name)) {
lastName = name;
try {
//will be interrupted at first time
Thread.sleep(200);
} catch (InterruptedException ignore) {
}
}
return "Hello " + name + ", response from provider: " + RpcContext.getContext().getLocalAddress();
} important parameter:retries=3&cluster=failover flag will return normalReturn if we do not add |
@happytimor Please apply this pr to 3.0 branch. |
What is the purpose of the change
Interrupt current thread when InterruptException occur
org.apache.dubbo.rpc.protocol.AsyncToSyncInvoker.java
Brief changelog
Interrupt current thread when InterruptException occur
Verifying this change
Checklist