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

cherrypick-1.1: distsql: pre-reserve memory needed to mark rows in HashJoiner build phase #18975

Merged
merged 1 commit into from Oct 3, 2017

Conversation

Projects
None yet
3 participants
@asubiotto
Contributor

asubiotto commented Oct 3, 2017

A situation was uncovered by #18600, where the HashJoiner would run out
of memory in the probe phase. This was because we had made an assumption
that we wouldn't hit a memory limit if the buffer phase filled up at
most 2/3 of the limit with both streams , since the marking
infrastructure would take up only a fraction of 1/3 (the chosen stream).

This assumption failed to take into account other limits shared with
other processors. This change pre-reserves the memory needed for the
probe phase in the build phase so that we can keep a single point in the
code where we fall back to disk while not relying on any limit
assumptions.

distsql: pre-reserve memory needed to mark rows in HashJoiner build p…
…hase

A situation was uncovered by #18600, where the HashJoiner would run out
of memory in the probe phase. This was because we had made an assumption
that we wouldn't hit a memory limit if the buffer phase filled up at
most 2/3 of the limit with both streams , since the marking
infrastructure would take up only a fraction of 1/3 (the chosen stream).

This assumption failed to take into account other limits shared with
other processors. This change pre-reserves the memory needed for the
probe phase in the build phase so that we can keep a single point in the
code where we fall back to disk while not relying on any limit
assumptions.

@asubiotto asubiotto requested a review from andreimatei Oct 3, 2017

@asubiotto asubiotto requested review from cockroachdb/distsql-prs as code owners Oct 3, 2017

@cockroach-teamcity

This comment has been minimized.

Show comment
Hide comment
@cockroach-teamcity

cockroach-teamcity Oct 3, 2017

Member

This change is Reviewable

Member

cockroach-teamcity commented Oct 3, 2017

This change is Reviewable

@asubiotto asubiotto requested a review from cockroachdb/release Oct 3, 2017

@andreimatei

This comment has been minimized.

Show comment
Hide comment
@andreimatei

andreimatei Oct 3, 2017

Member

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

Member

andreimatei commented Oct 3, 2017

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@asubiotto asubiotto merged commit 386d96f into cockroachdb:release-1.1 Oct 3, 2017

4 checks passed

ci/teamcity TeamCity build finished
Details
codacy/pr Good work! A positive pull request.
Details
code-review/reviewable Review complete: 1 of 0 LGTMs obtained
Details
license/cla Contributor License Agreement is signed.
Details

@asubiotto asubiotto deleted the asubiotto:hashjoiner-1.1 branch Oct 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment