You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2016/02/29 07:19:21 UTC

[Impala-CR](cdh5-2.2.0_5.4.x) IMPALA-1702: Fix crash when IMPALA-1702 hit even with workaround patch

Hello Internal Jenkins, Skye Wanderman-Milne,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: IMPALA-1702: Fix crash when IMPALA-1702 hit even with workaround patch
......................................................................

IMPALA-1702: Fix crash when IMPALA-1702 hit even with workaround patch

IMPALA-1702 results in bad partition indexes. This would cause Impala to crash as the
partition descriptor object for that index would be NULL, and Impala would happily try to
dereference it.

To provide a workaround, we added a patch which would not DCHECK, but would gracefully
return when this condition was detected. However, the graceful exit wasn't extended to
HdfsScanNode::Close(), which is called after Prepare() even if Prepare() failed.

This patch fixes the issue by ensuring that Close() also gracefully
fails.

To defensively protect against the situation where it may not be safe
to Close() an ExprContext that hasn't been Prepare()'d, this patch adds
a guard to HdfsPartitionDescriptor::CloseExprs() to ensure that only
Prepare()'d exprs are Close()'d. This is necessary in this patch for the
same reasons as the crash above: the scan node may Close() after an
unsuccessful Prepare().

IMPALA-2156 is related, as some of this defensive code would be easier
if the lifecycle of exprs was better specified.

Testing was manual, by editing the partition metadata to inject bad partition indexes.

Change-Id: I3d5a20871ab83dfa2d6aae85ca6610ddee93daac
Reviewed-on: http://gerrit.cloudera.org:8080/534
Reviewed-by: Skye Wanderman-Milne <sk...@cloudera.com>
Tested-by: Internal Jenkins
(cherry picked from commit 154830d919bc972ccad49628ab2c6d99addc8dfd)
(cherry picked from commit d457860aa4f5f3c8116dac85301890948d8cb3b7)
---
M be/src/exec/hdfs-scan-node.cc
M be/src/runtime/descriptors.cc
2 files changed, 9 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/50/2350/1
-- 
To view, visit http://gerrit.cloudera.org:8080/2350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d5a20871ab83dfa2d6aae85ca6610ddee93daac
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.2.0_5.4.x
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Skye Wanderman-Milne <sk...@cloudera.com>