You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/11/21 22:17:01 UTC

[Impala-ASF-CR] IMPALA-9126: part 2: no hash join probe structures in build

Hello Michael Ho, 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 (#9).

Change subject: IMPALA-9126: part 2: no hash join probe structures in build
......................................................................

IMPALA-9126: part 2: no hash join 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/9
-- 
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: 9
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>