[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 8: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14716/8//COMMIT_MSG@7 PS8, Line 7: part 2: no hj probe structures in build nit: I would prefer to spell out hash join to make it clearer to people who are just browsing git log --oneline, e.g. "Remove probe structures from hash join builder". -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:39:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5102/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:04:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5101/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 20:02:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/7/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14716/7/be/src/exec/partitioned-hash-join-builder.h@128 PS7, Line 128: int GetNumSpilledHashPartitions() const; This no longer needs to be public. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 Nov 2019 19:42:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Hello Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#8). Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 185 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/8 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Hello Thomas Tauber-Marshall, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#6). Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 186 insertions(+), 179 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/6 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14716 )
Change subject: IMPALA-9126: part 2: no hj probe structures in build
..
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@118
PS5, Line 118: /// new partitions with level input_partition->leve() + 1. The
previous hash partitions
> typo: missing "l"
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@329
PS5, Line 329: m
> nit: capital M
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@483
PS5, Line 483: //
> nit: missing /
Done
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@987
PS5, Line 987: probe_hash_partitions_.resize(PARTITION_FANOUT);
: bool have_spilled_hash_partitions = false;
: for (int i = 0; i < PARTITION_FANOUT; ++i) {
: PhjBuilder::Partition* build_partition =
builder_->hash_partition(i);
: if (build_partition->IsClosed() ||
!build_partition->is_spilled()) continue;
: have_spilled_hash_partitions = true;
: DCHECK(!probe_streams.empty()) << "Builder should have
created enough streams";
: CreateProbePartition(i, std::move(probe_streams.back()));
: probe_streams.pop_back();
: }
: DCHECK(probe_streams.empty()) << "Builder should not have
created extra streams";
> This loop and AllocateProbeStreams() feel redundant - CreateProbePartition
That's a good point. I ended up restructuring this so that it's more consistent
with other code - it creates the Partitions in-place in probe_hash_partitions_
and initialises them with a PrepareForWrite() method. Much more concise.
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@1055
PS5, Line 1055: for (auto& stream : *streams) {
: stream->Close(nullptr,
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
: }
> I would prefer to avoid the gotos by doing cleanup on the caller side or ad
This got fixed as a consequence of the other refactoring.
--
To view, visit http://gerrit.cloudera.org:8080/14716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d
Gerrit-Change-Number: 14716
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 21 Nov 2019 19:33:08 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/14716 )
Change subject: IMPALA-9126: part 2: no hj probe structures in build
..
Patch Set 5: Code-Review+1
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@118
PS5, Line 118: /// new partitions with level input_partition->leve() + 1. The
previous hash partitions
typo: missing "l"
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@329
PS5, Line 329: m
nit: capital M
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-builder.h@483
PS5, Line 483: //
nit: missing /
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@987
PS5, Line 987: probe_hash_partitions_.resize(PARTITION_FANOUT);
: bool have_spilled_hash_partitions = false;
: for (int i = 0; i < PARTITION_FANOUT; ++i) {
: PhjBuilder::Partition* build_partition =
builder_->hash_partition(i);
: if (build_partition->IsClosed() ||
!build_partition->is_spilled()) continue;
: have_spilled_hash_partitions = true;
: DCHECK(!probe_streams.empty()) << "Builder should have
created enough streams";
: CreateProbePartition(i, std::move(probe_streams.back()));
: probe_streams.pop_back();
: }
: DCHECK(probe_streams.empty()) << "Builder should not have
created extra streams";
This loop and AllocateProbeStreams() feel redundant - CreateProbePartition
doesn't seem to do anything fancy, it just constructs a ProbePartition without
side effects.
I am not sure about the best way to merge them - maybe CreateProbePartition
could handle both the allocation and the ProbePartition construction + the
cleanup on error stuff could be done using the members of
probe_hash_partitions_.
http://gerrit.cloudera.org:8080/#/c/14716/5/be/src/exec/partitioned-hash-join-node.cc@1055
PS5, Line 1055: for (auto& stream : *streams) {
: stream->Close(nullptr,
RowBatch::FlushMode::NO_FLUSH_RESOURCES);
: }
I would prefer to avoid the gotos by doing cleanup on the caller side or adding
a wrapper function.
--
To view, visit http://gerrit.cloudera.org:8080/14716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d
Gerrit-Change-Number: 14716
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Thu, 21 Nov 2019 18:10:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5076/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 19 Nov 2019 23:28:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5075/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 19 Nov 2019 22:50:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has restored this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Restored -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14716 to look at the new patch set (#5). Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 176 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/5 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14716/4/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/14716/4/be/src/exec/partitioned-hash-join-builder.cc@414 PS4, Line 414: bool need_read_buffer = ht_ctx_->level() > 0; I thought this was a bug, because we need a read buffer when we're probing a single spilled partition, even if that partition's level is 0. But actually it works for a non-obvious reason - we don't currently call this function when we're probing a single spilled partition. Changed the code around so this is more explicit. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 19 Nov 2019 22:44:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has abandoned this change. ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. Abandoned Noticed a bug - will abandon for now. -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9126: part 2: no hj probe structures in build
Tim Armstrong has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/14716 ) Change subject: IMPALA-9126: part 2: no hj probe structures in build .. IMPALA-9126: part 2: no hj probe structures in build This is actually independent of part 1, and can be merged ahead of it if needed. This cleans a up a bit of tech debt, where the hash join builder allocated probe-side streams. This was implemented before we had reliable memory reservations. Now we can simply transfer reservation. The reason things are this way is because the separation of PhjBuilder from PartitionedHashJoinNode (IMPALA-3567) happened before we switched to the new BufferPool (IMPALA-4674). It wasn't possible to reliably transfer reservations, instead the workaround of allocating and transferring probe streams was necessary. After this change, PartitionedHashJoinBuilder does not explicitly touch any probe-side data structures. There is still some implicit sharing of things like the buffer pool client, which is expected as long as the builder belongs to the ExecNode. Testing: Ran exhaustive tests. We should already have adequate coverage for spilling and non-spilling hash joins. Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h 6 files changed, 175 insertions(+), 131 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/14716/4 -- To view, visit http://gerrit.cloudera.org:8080/14716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0065f7f44f44f02b7616b1f694178ca42341c42d Gerrit-Change-Number: 14716 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong
