You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "TangSiyang2001 (via GitHub)" <gi...@apache.org> on 2023/02/25 13:05:22 UTC
[GitHub] [doris] TangSiyang2001 opened a new pull request, #17132: [Enhancement](stmt-forward) better error msg for follower fe
TangSiyang2001 opened a new pull request, #17132:
URL: https://github.com/apache/doris/pull/17132
# Proposed changes
Issue Number: close #16300
## Problem summary
The error log msg for the FE follower's forward to master failure is ambiguous as seen, so we should clarify it.


## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [x] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445411949
sql client log
```
mysql> insert into lineorder select * from lineorder limit 10000000;
ERROR 1105 (HY000): ForwardToMasterException, msg: org.apache.doris.qe.MasterOpExecutor$ForwardToMasterException: Forward statement 21 to Master TNetworkAddress(hostname:127.0.0.1, port:9020) failed, cause: Unknown exception
```
fe log
```
2023-02-27 01:02:15,733 WARN (mysql-nio-pool-6|140) [StmtExecutor.execute():630] execute Exception. stmt[22, 9bed44b1950b4351-b003c1e13820c44e]
org.apache.doris.qe.MasterOpExecutor$ForwardToMasterException: Forward statement 22 to Master TNetworkAddress(hostname:127.0.0.1, port:9020) failed, cause: Unknown exception
at org.apache.doris.qe.MasterOpExecutor.forward(MasterOpExecutor.java:137) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.MasterOpExecutor.execute(MasterOpExecutor.java:73) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.StmtExecutor.forwardToMaster(StmtExecutor.java:695) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.StmtExecutor.execute(StmtExecutor.java:497) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.StmtExecutor.execute(StmtExecutor.java:425) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.ConnectProcessor.handleQuery(ConnectProcessor.java:415) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.ConnectProcessor.dispatch(ConnectProcessor.java:557) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.qe.ConnectProcessor.processOnce(ConnectProcessor.java:794) ~[doris-fe.jar:1.2-SNAPSHOT]
at org.apache.doris.mysql.ReadListener.lambda$handleEvent$0(ReadListener.java:52) ~[doris-fe.jar:1.2-SNAPSHOT]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[?:1.8.0_352]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) ~[?:1.8.0_352]
at java.lang.Thread.run(Thread.java:750) ~[?:1.8.0_352]
```
However, since the socket timeout exception will be classified as type: `unkonwn exception` in thrift, rather than `conn timeout` in reality, the result will not be `conn timeout` still. But If we use `conn timeout` msg to cover the `unkonwn exception`, that may hide the msg of other `unkonwn exception`s, should I enhance it further @freemandealer ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445121685
run p0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on a diff in pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on code in PR #17132:
URL: https://github.com/apache/doris/pull/17132#discussion_r1118011505
##########
fe/fe-core/src/main/java/org/apache/doris/qe/MasterOpExecutor.java:
##########
@@ -127,19 +129,28 @@ private void forward() throws Exception {
boolean isReturnToPool = false;
try {
- result = client.forward(params);
+ client.forward(params);
isReturnToPool = true;
} catch (TTransportException e) {
+ // wrap the raw exception.
+ Exception exception = new ForwardToMasterException(
+ String.format("Forward statement %s to Master %s failed", ctx.getStmtId(),
+ thriftAddress), e);
+
boolean ok = ClientPool.frontendPool.reopen(client, thriftTimeoutMs);
if (!ok) {
- throw e;
+ throw exception;
}
if (shouldNotRetry || e.getType() == TTransportException.TIMED_OUT) {
- throw e;
+ throw exception;
} else {
LOG.warn("Forward statement " + ctx.getStmtId() + " to Master " + thriftAddress + " twice", e);
- result = client.forward(params);
- isReturnToPool = true;
+ try {
+ client.forward(params);
+ isReturnToPool = true;
+ } catch (TException ex) {
Review Comment:
That may result in a naming confict haha.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445260548
PR approved by anyone and no changes requested.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445638788
Hi @TangSiyang2001 You would better make it clearer. Our purpose is to show the user the root cause of the problem as clearly as we can, instead of a superficial java exception. At least, the error logs from the follower FE (forwarded) should be the same as those directly from the master FE.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445263230
Hi @TangSiyang2001 , Could you please use text instead of image to show the issue?
And is there any show case after your modification?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Yukang-Lian commented on a diff in pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "Yukang-Lian (via GitHub)" <gi...@apache.org>.
Yukang-Lian commented on code in PR #17132:
URL: https://github.com/apache/doris/pull/17132#discussion_r1117960780
##########
fe/fe-core/src/main/java/org/apache/doris/qe/MasterOpExecutor.java:
##########
@@ -127,19 +129,28 @@ private void forward() throws Exception {
boolean isReturnToPool = false;
try {
- result = client.forward(params);
+ client.forward(params);
isReturnToPool = true;
} catch (TTransportException e) {
+ // wrap the raw exception.
+ Exception exception = new ForwardToMasterException(
+ String.format("Forward statement %s to Master %s failed", ctx.getStmtId(),
+ thriftAddress), e);
+
boolean ok = ClientPool.frontendPool.reopen(client, thriftTimeoutMs);
if (!ok) {
- throw e;
+ throw exception;
}
if (shouldNotRetry || e.getType() == TTransportException.TIMED_OUT) {
- throw e;
+ throw exception;
} else {
LOG.warn("Forward statement " + ctx.getStmtId() + " to Master " + thriftAddress + " twice", e);
- result = client.forward(params);
- isReturnToPool = true;
+ try {
+ client.forward(params);
+ isReturnToPool = true;
+ } catch (TException ex) {
Review Comment:
maybe use `e` so that the format is consistent with all exception?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei merged pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #17132:
URL: https://github.com/apache/doris/pull/17132
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445121649
run buildall
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445265713
> > Thanks for your contribution. As a gentle reminder, please show us the error msg output after applying this fix.
>
> Maybe you can try this:
>
> 1. Start two fe processes, and then `show frontends` to see which one is is the follower
>
> 2. set insert into timeout short enough to let the backend fail at timeout
>
> 3. Send an insert into SQL to the follower fe. When it times out, you should see the error
Thank you for the guidance, I'll work in this way.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445264146
> Thanks for your contribution. As a gentle reminder, please show us the error msg output after applying this fix.
Maybe you can try this:
1) Start two fe processes, and then `show frontends` to see which one is is the follower
2) set insert into timeout short enough to let the backend fail at timeout
3) Send an insert into SQL to the follower fe. When it times out, you should see the error
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445265620
> Hi @TangSiyang2001 , Could you please use text instead of image to show the issue? And is there any show case after your modification?
Sure, I'll modify the descriptions and get the show case.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445263742
Thanks for your contribution. As a gentle reminder, please show us the error msg output after applying this fix.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445117016
run p0
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] TangSiyang2001 commented on pull request #17132: [Enhancement](stmt-forward) better error msg for follower fe
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #17132:
URL: https://github.com/apache/doris/pull/17132#issuecomment-1445116988
run buildall
--
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.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org