Skip to content

Conversation

@MorningLight5
Copy link
Contributor

Proposed changes

Fix #7129

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Code refactor (Modify the code structure, format the code, etc...)
  • Optimization. Including functional usability improvements and performance improvements.
  • Dependency. Such as changes related to third-party components.
  • Other.

Checklist

  • I have created an issue on (Fix #ISSUE) and described the bug/feature there in detail
  • Compiling and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • If these changes need document changes, I have updated the document
  • Any dependent changes have been merged

@xinyiZzz
Copy link
Contributor

xinyiZzz commented Dec 23, 2021

Good idea,
If FE can get the memory consumption of each Query and Load, we can limit the memory used by users, which is what I plan to do in the future.

Impala’s AdmissionController does a similar thing, Introduction is here https://shimo.im/docs/6qxjctpyDHJgPwtw

In #7198, I have counted the real memory consumption of each Query, which may be implemented based on your PR later

@morningman
Copy link
Contributor

Hi @tianhui5 , thanks for your contribution.
But better to add document to explain the usage or best practice.
Otherwise, it is hard to review.

@morningman morningman self-assigned this Dec 23, 2021
@morningman morningman added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 23, 2021
@MorningLight5 MorningLight5 changed the title [Feature] Limit cluster resource usage in user granularity #7129 [Feature] Limit frequency of operation in both cluster and user granularity #7129 Dec 24, 2021
@MorningLight5 MorningLight5 changed the title [Feature] Limit frequency of operation in both cluster and user granularity #7129 [Feature] Limit frequency of operation in both cluster and user granularity Dec 24, 2021
MorningLight5 added a commit to MorningLight5/incubator-doris that referenced this pull request Jan 4, 2022
@MorningLight5
Copy link
Contributor Author

Hi @tianhui5 , thanks for your contribution. But better to add document to explain the usage or best practice. Otherwise, it is hard to review.

Thanks for remind me. I've add relative doc and unit test, please review this PR

@MorningLight5
Copy link
Contributor Author

MorningLight5 commented Jan 4, 2022

Good idea, If FE can get the memory consumption of each Query and Load, we can limit the memory used by users, which is what I plan to do in the future.

Impala’s AdmissionController does a similar thing, Introduction is here https://shimo.im/docs/6qxjctpyDHJgPwtw

In #7198, I have counted the real memory consumption of each Query, which may be implemented based on your PR later

Nice work.
In my PR, I've limit the system usage through a general way, like limit QPS. We can develop a more concise way in next move as you say.

MorningLight5 added a commit to MorningLight5/incubator-doris that referenced this pull request Jan 5, 2022
@morningman
Copy link
Contributor

There are two main issues

  1. all query requests need to send an RPC to Master FE, which is too much overhead.
  2. Limiter is not set in the right place. For example, for query, it should be handled at the place where the query plan is finally sent to BE. For load, the frequency of transactions should be limited.

MorningLight5 added a commit to MorningLight5/incubator-doris that referenced this pull request Jan 29, 2022
@MorningLight5
Copy link
Contributor Author

There are two main issues

  1. all query requests need to send an RPC to Master FE, which is too much overhead.
  2. Limiter is not set in the right place. For example, for query, it should be handled at the place where the query plan is finally sent to BE. For load, the frequency of transactions should be limited.

Yes, that seems to have too much RPC. I have rewrote the feature with statistic sychronize based on bdbje. Because I think it's fine to use bdbje to store some temperory statistics, and there is too much work to develop a middleware myself.

Please review this patch, thanks for your time!

@MorningLight5 MorningLight5 changed the title [Feature] Limit frequency of operation in both cluster and user granularity [Feature] Limit frequency of query and load in system level Jan 29, 2022
@morningman
Copy link
Contributor

Hi @tianhui5

Although we had some private discussions about this feature, we eventually needed a complete design and usage document to help all developers and users understand how this feature works and how to use it.

It is recommended that the final design be fully described in the issue #7129 (and the previous design be marked as Deprecated).

Also, I suggest a separate user document describing the functionality from a usage perspective, including the followings:

  1. the purpose of the feature (Motivation)
  2. what scenarios the feature is applicable to, how to use it, and what impact it will have.
  3. whether there are shortcomings for further improvement.

In the previous Doris development, we also had a lot of features that were difficult to maintain due to lack of documentation, and I hope to be able to improve the process in this area in future projects. Thanks!

@MorningLight5
Copy link
Contributor Author

MorningLight5 commented Feb 14, 2022

Hi @tianhui5

Although we had some private discussions about this feature, we eventually needed a complete design and usage document to help all developers and users understand how this feature works and how to use it.

It is recommended that the final design be fully described in the issue #7129 (and the previous design be marked as Deprecated).

Also, I suggest a separate user document describing the functionality from a usage perspective, including the followings:

  1. the purpose of the feature (Motivation)
  2. what scenarios the feature is applicable to, how to use it, and what impact it will have.
  3. whether there are shortcomings for further improvement.

In the previous Doris development, we also had a lot of features that were difficult to maintain due to lack of documentation, and I hope to be able to improve the process in this area in future projects. Thanks!

Done.

@MorningLight5
Copy link
Contributor Author

The local current query number is reported to Master every certain time, and then send to all FE.
image

@github-actions
Copy link
Contributor

We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Aug 18, 2022
@github-actions github-actions bot closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature. Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Limit cluster resource usage in user granularity

3 participants