[
https://issues.apache.org/jira/browse/IMPALA-9127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16982666#comment-16982666
]
ASF subversion and git services commented on IMPALA-9127:
---------------------------------------------------------
Commit 8e33b84b049e9d31188792ae18ad43c0e887966b in impala's branch
refs/heads/master from Tim Armstrong
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=8e33b84 ]
IMPALA-9127: explicit probe state machine in hash join
This refactors the main loop in PartitionedHashJoinNode::GetNext()
to use an explicit state machine, rather than the hard-to-follow
implicit state machine previously used. A new state variable
'probe_state_' is used to drive the loop, with DCHECKs enforcing
invariants of other member variables.
I deliberately tried to minimise changes to other functions
(including any attempts to factor logic out of GetNext())
to minimise the scope of this patch.
The new logic is mostly equivalent to the old logic, although there
may be a different number of trips through the loop because of the
way the cascading checks in the old version worked. A few notable
changes:
* DoneProbing() is consistently called when probing is finished,
including in cases, like probing a single spilled partition, where
it wasn't previously.
* The repeated AtCapacity() checks are consolidated into a single
check that happens at the end of the loop. Resources attached
to batches should still be flushed at the appropriate points,
since each previous "if (out_batch->AtCapacity()) break;"
corresponds to a new loop iteration in the new code.
* OutputNullAwareNullProbe() and OutputNullAwareProbeRows() now
explicitly signal when they are done using an output argument,
instead of implicitly via AtCapacity(), which is incredibly
error-prone.
Testing:
We have adequate coverage for different join modes, including
with spilling.
* Ran exhaustive tests.
* Ran a single node stress test with TPC-H and TPC-DS
* Ran a single node stress test with larger scale factor
Change-Id: I32ebdf0054d2ce4562b851439e300323601fb064
Reviewed-on: http://gerrit.cloudera.org:8080/14688
Reviewed-by: Tim Armstrong <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
> Clean up probe-side state machine in hash join
> ----------------------------------------------
>
> Key: IMPALA-9127
> URL: https://issues.apache.org/jira/browse/IMPALA-9127
> Project: IMPALA
> Issue Type: Sub-task
> Components: Backend
> Reporter: Tim Armstrong
> Assignee: Tim Armstrong
> Priority: Major
>
> There's an implicit state machine in the main loop in
> PartitionedHashJoinNode::GetNext()
> https://github.com/apache/impala/blob/eea617b/be/src/exec/partitioned-hash-join-node.cc#L510
> The state is implicitly defined based on the following conditions:
> * !output_build_partitions_.empty() -> "outputting build rows after probing"
> * builder_->null_aware_partition() == NULL -> "eos, because this the
> null-aware partition is processed after all other partitions"
> * null_probe_output_idx_ >= 0 -> "null probe rows being processed"
> * output_null_aware_probe_rows_running_ -> "null-aware partition being
> processed"
> * probe_batch_pos_ != -1 -> "processing probe batch"
> * builder_->num_hash_partitions() != 0 -> "have active hash partitions that
> are being probed"
> * spilled_partitions_.empty() -> "no more spilled partitions"
> I think this would be a lot easier to follow if the state machine was
> explicit and documented, and would make separating out the build side of a
> spilling hash join easier to get right.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
