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 memory accounting via HTTP interface #11840

Merged
merged 5 commits into from Jun 23, 2020

Conversation

azat
Copy link
Collaborator

@azat azat commented Jun 21, 2020

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix memory accounting via HTTP interface (can be significant with wait_end_of_query=1)

Fixes: #11153
Similar to: #11022

azat added 2 commits June 21, 2020 17:27
function perf_test()
{
    time yes '127.1:8123/?wait_end_of_query=1' | head -n10000 | xargs -P10000 curl -s -d 'select 1' | grep -x -c 1
}
function server()
{
    local limit=$1 && shift
    clickhouse-server "$@" -- --max_server_memory_usage=$limit
}

perf_test:

- before this patch with 1G  limit: succeed only ~800  queries
- after  this patch with 1G  limit: succeed      ~8000 queries

- before this patch with 10G limit: succeed only ~3000  queries
- after  this patch with 10G limit: succeed      ~10000 queries

Fixes: ClickHouse#11153
@blinkov blinkov added the pr-bugfix Pull request with bugfix, not backported by default label Jun 21, 2020
@alexey-milovidov
Copy link
Member

Docs check — Found 1 errors

Broken in master @blinkov

@alexey-milovidov alexey-milovidov self-assigned this Jun 21, 2020
@alexey-milovidov
Copy link
Member

Something wrong with 01091_num_threads test.

@alexey-milovidov
Copy link
Member

#9483

@alexey-milovidov
Copy link
Member

But the way, this test is also flaky but it fails only occasionally.

@alexey-milovidov alexey-milovidov added the st-waiting-for-fix We are waiting for fixes in this issue or pull request label Jun 21, 2020
@azat
Copy link
Collaborator Author

azat commented Jun 21, 2020

But the way, this test is also flaky but it fails only occasionally.

I don't see the failure, but it may require some tuning

@azat azat force-pushed the fix-http-memory-accounting branch 4 times, most recently from e183f5a to 1b1b940 Compare June 22, 2020 09:01
w/o the patch the test fails with, since it succeed only 512 queries.
@azat azat force-pushed the fix-http-memory-accounting branch from 1b1b940 to 9744c99 Compare June 22, 2020 09:02
@azat
Copy link
Collaborator Author

azat commented Jun 22, 2020

But the way, this test is also flaky but it fails only occasionally.

I don't see the failure, but it may require some tuning

@alexey-milovidov Ok, found, and fixed and now only 01091_num_threads fails.

@alexey-milovidov
Copy link
Member

We have to ask @KochetovNicolai for help or maybe you can investigate?

Actually there were two issues:
- missing \n, so it picked the wrong query anway
- no unique identifier, hence it may works incorrectly (and actually,
  event clickhouse-test script executes 'SELECT 1' query)
@azat
Copy link
Collaborator Author

azat commented Jun 22, 2020

We have to ask @KochetovNicolai for help or maybe you can investigate?

Interesting, fixed - 267a6c8

P.S. Hm, I didn't look that it does not fails on upstream, egh.

@azat
Copy link
Collaborator Author

azat commented Jun 23, 2020

Now tests passed

@alexey-milovidov alexey-milovidov merged commit b447508 into ClickHouse:master Jun 23, 2020
@azat azat deleted the fix-http-memory-accounting branch June 23, 2020 16:38
abyss7 added a commit that referenced this pull request Jun 25, 2020
…ceff19401bbc5b99b1716934b2827

Cherry pick #11840 to 20.5: Fix memory accounting via HTTP interface
alexey-milovidov added a commit that referenced this pull request Jul 7, 2020
…1968)

Co-authored-by: robot-clickhouse <robot-clickhouse@yandex-team.ru>
Co-authored-by: alexey-milovidov <milovidov@yandex-team.ru>
alesapin pushed a commit that referenced this pull request Jul 10, 2020
alesapin added a commit that referenced this pull request Jul 10, 2020
alesapin pushed a commit that referenced this pull request Jul 10, 2020
Fix memory accounting via HTTP interface

(cherry picked from commit b447508)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default st-waiting-for-fix We are waiting for fixes in this issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory limit (total) exceeded issues with wait_end_of_query=1
5 participants