Skip to content

23.3 Backport of #50429 - Fix excessive memory usage for FINAL (due to too much streams usage)#307

Merged
Enmk merged 1 commit into
customizations/23.3.8from
backports/23.3.8/50429_fix-final-memory-usage
Sep 21, 2023
Merged

23.3 Backport of #50429 - Fix excessive memory usage for FINAL (due to too much streams usage)#307
Enmk merged 1 commit into
customizations/23.3.8from
backports/23.3.8/50429_fix-final-memory-usage

Conversation

@Enmk

@Enmk Enmk commented Sep 20, 2023

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix excessive memory usage for FINAL (due to too much streams usage) (ClickHouse#50429 by @azat)

@azat @Enmk
Fix excessive memory usage for FINAL (due to too much streams usage) (C…
a5c9dc5 
…lickHouse#50429)

Previously it could create MergeTreeInOrder for each mark, however this
could be very suboptimal, due to each MergeTreeInOrder has some memory
overhead.

Now, by collapsing all marks for one part together it is more memory
effiecient.

I've tried the query from the altinity wiki [1] and it decreases memory
usage twice:

    SELECT * FROM repl_tbl FINAL WHERE key IN (SELECT toUInt32(number) FROM numbers(1000000) WHERE number % 50000 = 0) FORMAT Null

- upstream: MemoryTracker: Peak memory usage (for query): 520.27 MiB.
- patched:  MemoryTracker: Peak memory usage (for query): 260.95 MiB.

  [1]: https://kb.altinity.com/engines/mergetree-table-engine-family/replacingmergetree/#multiple-keys

And it could be not 2x and even more or less, it depends on the gaps in
marks for reading (for example in my setup the memory usage increased a
lot, from ~16GiB of RAM to >64GiB due to lots of marks and gaps).

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@altinity-robot

altinity-robot commented Sep 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This is an automated comment for commit a5c9dc5 with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🔴 failure

Check nameDescriptionStatus
CI runningA meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR🟡 pending
ClickHouse build checkBuilds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process🟢 success
Compatibility checkChecks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help🔴 failure
Docker image for serversThe check to build and optionally push the mentioned image to docker hub🟢 success
Install packagesChecks that the built packages are installable in a clear environment🟢 success
Integration testsThe integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests🔴 failure
Mergeable CheckChecks if all other necessary checks are successful🟢 success
Push to DockerhubThe check for building and pushing the CI related docker images to docker hub🟢 success
Ready for releaseThere's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS🟢 success
Stateful testsRuns stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc🟢 success
Stateless testsRuns stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc🟢 success

@Enmk Enmk changed the title 23.3 Backport of #50429 Fix excessive memory usage for FINAL (due to too much streams usage) 23.3 Backport of #50429 - Fix excessive memory usage for FINAL (due to too much streams usage) Sep 21, 2023
@Enmk Enmk merged commit c709951 into customizations/23.3.8 Sep 21, 2023
This was referenced Sep 25, 2023
Merged
Merged
@Enmk Enmk mentioned this pull request Mar 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Enmk @altinity-robot @azat