Skip to content
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

[fix](timeout) query timeout was not correctly set #33444

Merged
merged 3 commits into from
Apr 12, 2024

Conversation

Mryange
Copy link
Contributor

@Mryange Mryange commented Apr 9, 2024

Proposed changes

In the past, in ConnectContext's checkTimeout, if a timeout occurred and cancel was not passed PPlanFragmentCancelReason, it resulted in being set to Types.PPlanFragmentCancelReason.USER_CANCEL.

    public void cancel() {
        cancel(Types.PPlanFragmentCancelReason.USER_CANCEL);
    }
2024-03-29 13:11:11,683 WARN (connect-scheduler-check-timer-0|103) [ConnectContext.checkTimeout():921] kill query timeout, remote: 172.30.0.32:50876, query timeout: 600000
2024-03-29 13:11:11,683 WARN (connect-scheduler-check-timer-0|103) [ConnectContext.kill():877] kill query from 172.30.0.32:50876, kill mysql connection: false
2024-03-29 13:10:39,686 WARN (connect-scheduler-check-timer-0|103) [Coordinator.cancel():1443] Cancel execution of query a82b68a3a1bc44b9-b4bc0254ac412006, this is a outside invoke
2024-03-29 13:10:39,698 WARN (mysql-nio-pool-67|868) [ResultReceiver.getNext():102] Query a82b68a3a1bc44b9-b4bc0254ac412006 get result timeout, get result duration 599 ms
2024-03-29 13:10:39,698 WARN (mysql-nio-pool-67|868) [ResultReceiver.updateCancelReason():194] Query a82b68a3a1bc44b9-b4bc0254ac412006 already has cancel reason: USER_CANCEL, new reason fetch data timeout will be ignored

The ResultReceiver was canceled before getNext was called. The original code only set status.setStatus(Status.CANCELLED);, so there was no way to report a timeout error, only CANCELLED.

2024-04-09 08:39:14,106 INFO (mysql-nio-pool-31|1394) [Coordinator.execInternal():681] dispatch result sink of query bb9431afba254d2f-81b33c36f9dc136e to TNetworkAddress(hostname:172.30.0.47, port:9060)
2024-04-09 08:49:14,899 WARN (connect-scheduler-check-timer-0|102) [Coordinator.cancel():1443] Cancel execution of query bb9431afba254d2f-81b33c36f9dc136e, this is a outside invoke, cancelReason TIMEOUT
2024-04-09 08:49:14,900 WARN (connect-scheduler-check-timer-0|102) [ResultReceiver.cancel():214] ResultReceiver of query bb9431afba254d2f-81b33c36f9dc136e cancel failed, typically means the future is finished
2024-04-09 08:49:15,952 WARN (mysql-nio-pool-31|1394) [Coordinator.getNext():1303] Query bb9431afba254d2f-81b33c36f9dc136e coordinator get next fail, Cancelled, need cancel.
2024-04-09 08:49:15,952 WARN (mysql-nio-pool-31|1394) [StmtExecutor.sendResult():1721] cancel fragment query_id:bb9431afba254d2f-81b33c36f9dc136e cause errCode = 2, detailMessage = Cancelled
2024-04-09 08:49:18,818 WARN (thrift-server-pool-34|1681) [QeProcessorImpl.reportExecStatus():226] ReportExecStatus() runtime error, query bb9431afba254d2f-81b33c36f9dc136e with type SELECT does not exist

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@Mryange
Copy link
Contributor Author

Mryange commented Apr 9, 2024

run buildall

@Mryange
Copy link
Contributor Author

Mryange commented Apr 9, 2024

run buildall

@Mryange
Copy link
Contributor Author

Mryange commented Apr 10, 2024

run buildall

@Mryange
Copy link
Contributor Author

Mryange commented Apr 10, 2024

run buildall

@Mryange
Copy link
Contributor Author

Mryange commented Apr 11, 2024

run buildall

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 11, 2024
Copy link
Contributor

PR approved by at least one committer and no changes requested.

Copy link
Contributor

PR approved by anyone and no changes requested.

@Gabriel39 Gabriel39 merged commit 50f0ca8 into apache:master Apr 12, 2024
25 of 29 checks passed
seawinde pushed a commit to seawinde/doris that referenced this pull request Apr 15, 2024
seawinde pushed a commit to seawinde/doris that referenced this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants