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](streamload&sink) release and allocate memory in the same tracker #12820

Merged
merged 6 commits into from Sep 23, 2022

Conversation

dataroaring
Copy link
Contributor

  1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.

  2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.

Proposed changes

Issue Number: close #xxx

Problem summary

Describe your changes.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • 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...

xinyiZzz
xinyiZzz previously approved these changes Sep 22, 2022
Copy link
Contributor

@xinyiZzz xinyiZzz 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 Sep 22, 2022
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions bot added kind/docs Categorizes issue or PR as related to documentation. and removed approved Indicates a PR has been approved by one committer. labels Sep 22, 2022
@dataroaring dataroaring force-pushed the stream_load_mem branch 2 times, most recently from 03bfdc8 to dd49c62 Compare September 22, 2022 11:35
xinyiZzz
xinyiZzz previously approved these changes Sep 22, 2022
Copy link
Contributor

@xinyiZzz xinyiZzz 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 Sep 22, 2022
@github-actions
Copy link
Contributor

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

1. HttpServer threads allocate bytebuffer and put them into streamload pipe,
but scanner thread release them with query tracker.

2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
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

@morningman morningman merged commit a7d42b5 into apache:master Sep 23, 2022
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
FreeOnePlus pushed a commit to FreeOnePlus/doris that referenced this pull request Oct 8, 2022
apache#12820)

1. HttpServer threads allocate bytebuffer and put them into streamload pipe, but scanner thread release them with query tracker.
2. We can assume brpc allocate memory in doris thread.

Above problems leads to wrong result of memtracker.
@yiguolei yiguolei mentioned this pull request Oct 9, 2022
yiguolei pushed a commit that referenced this pull request Mar 26, 2023
#11740 , solved the problem that the query memory statistics are higher than the actual physical memory, because PODArray does not have memset 0 when allocating memory, and the query mem tracker is virtual memory.

But in extreme cases, such as csv load, PODArray frequent insert will cause performance problems. So revert part of #11740 and part of #12820.

The accuracy of the query mem tracker, there is currently no feedback, no further attention.
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 21, 2023
…8010

apache#11740 , solved the problem that the query memory statistics are higher than the actual physical memory, because PODArray does not have memset 0 when allocating memory, and the query mem tracker is virtual memory.

But in extreme cases, such as csv load, PODArray frequent insert will cause performance problems. So revert part of apache#11740 and part of apache#12820.

The accuracy of the query mem tracker, there is currently no feedback, no further attention.
mongo360 pushed a commit to mongo360/doris that referenced this pull request Jul 12, 2023
…8010

apache#11740 , solved the problem that the query memory statistics are higher than the actual physical memory, because PODArray does not have memset 0 when allocating memory, and the query mem tracker is virtual memory.

But in extreme cases, such as csv load, PODArray frequent insert will cause performance problems. So revert part of apache#11740 and part of apache#12820.

The accuracy of the query mem tracker, there is currently no feedback, no further attention.
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Jul 28, 2023
…8010

apache#11740 , solved the problem that the query memory statistics are higher than the actual physical memory, because PODArray does not have memset 0 when allocating memory, and the query mem tracker is virtual memory.

But in extreme cases, such as csv load, PODArray frequent insert will cause performance problems. So revert part of apache#11740 and part of apache#12820.

The accuracy of the query mem tracker, there is currently no feedback, no further attention.
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. area/vectorization dev/merged-1.1.3-deprecated kind/docs Categorizes issue or PR as related to documentation. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants