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

Allocate before loading when responding to queries #255

Open
Stebalien opened this issue Oct 23, 2021 · 2 comments
Open

Allocate before loading when responding to queries #255

Stebalien opened this issue Oct 23, 2021 · 2 comments
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful P2 Medium: Good to have, but can wait until someone steps up

Comments

@Stebalien
Copy link
Member

In the query executor, we seem to load first, then allocate when sending the block. Unfortunately, when processing many parallel queries, we can end up loading a lot of blocks then blocking (waiting to be able to "reserve space" for them. we'd likely use less memory if we just sent the block.

Instead, we should allocate first. That is:

  1. Before loading anything, allocate 2-4MiB (maximum block size).
  2. Then load.
  3. Then reduce the allocation to the actual amount of memory needed.

This means we:

  1. Won't go over the allocation limit.
  2. If we're blocked on allocating, we won't sit there while holding on to buffers.
@Stebalien Stebalien added the need/triage Needs initial labeling and prioritization label Oct 23, 2021
@hannahhoward
Copy link
Collaborator

Yea this makes sense, but I'm not sure how much it will help us. My feeling is the massive spikes we're seeing in Estuary are not off by 1 loads but memory getting held for some reason.

It is still worth doing.

@hannahhoward hannahhoward added effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful P2 Medium: Good to have, but can wait until someone steps up and removed need/triage Needs initial labeling and prioritization labels Oct 26, 2021
@hannahhoward
Copy link
Collaborator

For anyone picking up this ticket: you will need to move use of the allocator around a bit to do this. You'll need to pipe it through as a dependency to the query executor, and possibly run traversal.

marten-seemann pushed a commit that referenced this issue Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

2 participants