Skip to content

[improvement](query) Improve fe high concurrent query performance #7936

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.

Sign up for GitHub

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

Jump to bottom
Merged
yangzhg merged 2 commits into from
Feb 8, 2022
Merged

[improvement](query) Improve fe high concurrent query performance #7936

yangzhg merged 2 commits into from
Feb 8, 2022

Conversation

@caiconghui
Copy link
Contributor

@caiconghui caiconghui commented Jan 30, 2022

Proposed changes

Issue Number: close #7935

Problem Summary:

Describe the overview of changes.

Checklist(Required)

  1. Does it affect the original behavior: (No)
  2. Has unit tests been added: (No)
  3. Has document been added or modified: (No Need)
  4. Does it need to update dependencies: (No Need)
  5. Are there any changes that cannot be rolled back: (No)

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...

@caiconghui
Copy link
Contributor Author

caiconghui commented Jan 30, 2022 •
edited
Loading

optimized statistic, 5k connection query 'select 1'
before tp99 561ms avg 262ms qps 15.56k
after tp99 406ms avg 130ms qps 32.2k

morningman
morningman reviewed Feb 6, 2022
View reviewed changes
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectScheduler.java
} else if (conns >= ctx.getCatalog().getAuth().getMaxConn(ctx.getQualifiedUser())) {
} else if (conns.incrementAndGet() > ctx.getCatalog().getAuth().getMaxConn(ctx.getQualifiedUser())) {
conns.decrementAndGet();
numberConnection.decrementAndGet();
Copy link
Contributor

@morningman morningman Feb 6, 2022

Choose a reason for hiding this comment

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

why call numberConnection.decrementAndGet(); here?

Copy link
Contributor Author

@caiconghui caiconghui Feb 6, 2022

Choose a reason for hiding this comment

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

why call numberConnection.decrementAndGet(); here?

we should reduce the connection number when connect failed

Copy link
Contributor

@morningman morningman Feb 7, 2022

Choose a reason for hiding this comment

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

Oh, I miss it.

fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java

LOG.info("dispatch query job: {} to {}", DebugUtil.printId(queryId), topParams.instanceExecParams.get(0).host);
if (LOG.isDebugEnabled()) {
LOG.debug("dispatch query job: {} to {}", DebugUtil.printId(queryId), topParams.instanceExecParams.get(0).host);
Copy link
Contributor

@morningman morningman Feb 6, 2022

Choose a reason for hiding this comment

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

Actually this log is useful when tracking a query. Does it really impact the performance?

Copy link
Contributor Author

@caiconghui caiconghui Feb 6, 2022

Choose a reason for hiding this comment

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

by the flame graph and the statistic provided above，for most users，query performance is the most important thing

morningman
morningman approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

fe/fe-core/src/main/java/org/apache/doris/qe/ConnectScheduler.java
} else if (conns >= ctx.getCatalog().getAuth().getMaxConn(ctx.getQualifiedUser())) {
} else if (conns.incrementAndGet() > ctx.getCatalog().getAuth().getMaxConn(ctx.getQualifiedUser())) {
conns.decrementAndGet();
numberConnection.decrementAndGet();
Copy link
Contributor

@morningman morningman Feb 7, 2022

Choose a reason for hiding this comment

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

Oh, I miss it.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

PR approved by anyone and no changes requested.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

PR approved by anyone and no changes requested.

@yangzhg yangzhg merged commit c6defb2 into apache:master Feb 8, 2022
@morningman morningman mentioned this pull request Feb 8, 2022
Merged
morningman added a commit that referenced this pull request Feb 9, 2022
@morningman
[improvement](log) Add query id info in error log for easy tracking (#…
df2c756 
…7975)

This PR #7936 change some FE log level to debug, so that when error happens, it is not easy to find out
which SQL cause the error.
So I add stmt id and query id in error log, so that user can use these identifiers to find SQL in fe.audit.log
@caiconghui caiconghui mentioned this pull request Feb 16, 2022
Merged
morningman pushed a commit that referenced this pull request Feb 19, 2022
@caiconghui
[fix](variables) Fix bug that execute showVariablesStmt with where ex…
9df5b2d 
…pression return empty resultset (#8094)

This Bug is introduced by PR #7936 , which change key type of connectionMap from Long to Integer,
which cause connectionMap could not find connectContext by connectionId
@caiconghui caiconghui deleted the high_performance branch March 12, 2022 01:31
Henry2SS pushed a commit to Henry2SS/incubator-doris that referenced this pull request Mar 16, 2022
@caiconghui
[improvement](query) Improve fe high concurrent query performance (ap…
d52cc4b 
…ache#7936)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morningman morningman morningman approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Enhancement] Improve fe high concurrent query performance

3 participants

@caiconghui @morningman @yangzhg