impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4098: Open()/Close() partition exprs once per fragment instance.
Date Fri, 09 Sep 2016 21:27:22 GMT
Hello Marcel Kornacker, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4340

to look at the new patch set (#3).

Change subject: IMPALA-4098: Open()/Close() partition exprs once per fragment instance.
......................................................................

IMPALA-4098: Open()/Close() partition exprs once per fragment instance.

Partition exprs stored in the descriptor table can be referenced by multiple
exec nodes (and/or a data sink) within the same fragment instance, so the
lifecycle of those exprs (Prepare/Open/Close) is tied to the fragment instance
and not to a particular exec node.

A recent change exposed this improper lifecycle management because we cloned
the partition exprs before using them, but by that time the exprs had been
closed which caused the cloning function to hit a DCHECK.

The fix is to tie the lifecycle of those exprs to that of the fragment
instance.

Testing: I could reliably reproduce the bug by running this query in a loop:

set num_nodes=1;
select count(a.year), count(a.month), count(a.int_col),
       count(b.year), count(b.month), count(b.int_col)
from functional.alltypessmall a, functional.alltypessmall b;

After this patch I was not able to reproduce the bug anymore. I don't think
it makes sense to add a test specifically for this bug because our existing
tests already caught it, and the hit DCHECK does not exist anymore due to
restructuring.

Change-Id: Id179df645e500530f4418988f6ce64a03d669892
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scanner.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M be/src/runtime/plan-fragment-executor.cc
6 files changed, 59 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/4340/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id179df645e500530f4418988f6ce64a03d669892
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>

Mime
View raw message