Skip to content

hash join: add build-side join keys to memory accounting - #14222

Closed
korowa wants to merge 1 commit into
apache:mainfrom
korowa:build-side-keys-mem
Closed

korowa wants to merge 1 commit into
apache:mainfrom
korowa:build-side-keys-mem

Conversation

@korowa

@korowa korowa commented Jan 21, 2025

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Follow-up on #14131

Rationale for this change

Having precalculated build-side join keys in JoinLeftData may be a noticeable overhead in terms of memory (always has been, but previously it's been recreated on each hashtable lookup), so it makes sense to add its memory into build-side data memory reservation.

What changes are included in this PR?

  • Build-side join keys memory is now accounted in memory reservation. These keys are now collected from build-side batches iterator to increase reserved memory size gradually (in order to reduce the risk of getting OOM killed), and concatenated later
  • Also due to arrow concat being non-consuming (and, thus, creating a copy of input data), concat operations for build-side join keys and record batches are now preceded by doubling up memory reservation for source data (trying to predict concatenated copy size), and followed by explicit drop of source data and resizing reservation to actual memory size of concat result.

Are these changes tested?

Existing test coverage, checking that build-side join keys evaluation not messed up.

Are there any user-facing changes?

No

@github-actions github-actions Bot added the physical-expr Changes to the physical-expr crates label Jan 21, 2025
@korowa

korowa commented Jan 21, 2025

Copy link
Copy Markdown
Contributor Author

cc @lewiszlw

@korowa
korowa force-pushed the build-side-keys-mem branch from 290e54b to 42e430e Compare January 21, 2025 18:05
@xudong963
xudong963 self-requested a review January 22, 2025 10:17
@alamb alamb mentioned this pull request Feb 16, 2025
@korowa
korowa force-pushed the build-side-keys-mem branch from 42e430e to 944e297 Compare February 18, 2025 05:10
@github-actions

Copy link
Copy Markdown

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions Bot added the Stale PR has not had any activity for some time label Apr 23, 2025
@github-actions github-actions Bot closed this May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates Stale PR has not had any activity for some time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant