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

feat(query): window partition by spill to disk #16441

Merged
merged 50 commits into from
Oct 8, 2024

Conversation

forsaken628
Copy link
Collaborator

@forsaken628 forsaken628 commented Sep 11, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

part of #15328

Changes:

  1. Local file direct io reading and writing
  2. Implement TempDirManager to provide local temporary file management, including usage control and cleanup.
  3. Add a new configuration item spill
  4. Window supports spill to local disk

How to enable:

  1. Check that the configuration item spill is configured correctly
  2. set window_partition_spilling_to_disk_bytes_limit = 10 * 1024 * 1024 * 1024 (10G), this is the maximum disk space allowed for each request, when the conditions of spill are met, the local disk will be used in preference

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label Sep 11, 2024
Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
@forsaken628 forsaken628 marked this pull request as ready for review September 14, 2024 04:44
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. A-query Area: databend query C-feature Category: feature labels Sep 14, 2024
@BohuTANG
Copy link
Member

Is there some benchmark numbers for this PR?

@forsaken628
Copy link
Collaborator Author

Is there some benchmark numbers for this PR?

@BohuTANG
Here just replace the storage, where can I go to find the right environment to do benchmark? If the test environment is not representative, benchmark has no meaning.

@BohuTANG
Copy link
Member

Is there some benchmark numbers for this PR?

@BohuTANG Here just replace the storage, where can I go to find the right environment to do benchmark? If the test environment is not representative, benchmark has no meaning.

I think @Dousir9 has some cases to do benchmark, because #16448 also need it.

Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
Signed-off-by: coldWater <[email protected]>
@Dousir9 Dousir9 removed this pull request from the merge queue due to a manual request Oct 8, 2024
@Dousir9 Dousir9 added this pull request to the merge queue Oct 8, 2024
@Dousir9 Dousir9 removed this pull request from the merge queue due to a manual request Oct 8, 2024
@forsaken628 forsaken628 added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels Oct 8, 2024
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Docker Image for PR

  • tag: pr-16441-31de655-1728375038

note: this image tag is only available for internal use,
please check the internal doc for more details.

@forsaken628 forsaken628 added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels Oct 8, 2024
Signed-off-by: coldWater <[email protected]>
@forsaken628 forsaken628 added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels Oct 8, 2024
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Docker Image for PR

  • tag: pr-16441-80d8a39-1728390166

note: this image tag is only available for internal use,
please check the internal doc for more details.

@Dousir9
Copy link
Member

Dousir9 commented Oct 8, 2024

After testing, it was found that the local spill that occurred on the local machine and the cloud had the problem of memory not being released in time, which led to performance degradation.

@forsaken628 forsaken628 added this pull request to the merge queue Oct 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 8, 2024
@Dousir9 Dousir9 added this pull request to the merge queue Oct 8, 2024
Merged via the queue into databendlabs:main with commit 040e8fa Oct 8, 2024
79 of 80 checks passed
@forsaken628 forsaken628 deleted the spill-disk branch October 9, 2024 01:29
@BohuTANG
Copy link
Member

BohuTANG commented Oct 9, 2024

After testing, it was found that the local spill that occurred on the local machine and the cloud had the problem of memory not being released in time, which led to performance degradation.

If so, this PR should revert from main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query Area: databend query C-feature Category: feature ci-cloud Build docker image for cloud test lgtm This PR has been approved by a maintainer pr-feature this PR introduces a new feature to the codebase size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants