[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-08-01 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..

IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter 
bank

While Prepare()-ing a fragment instance, if we fail to initialize the
runtime filter bank, we will exit FIS::Prepare() without acquiring a
thread token (AcquireThreadToken()):

FIS::Finalize() is called always regardless of whether the fragment
instance succeeded or failed. And FIS::Finalize() tries to ReleaseThreadToken()
even though it might not have gotten acquired, causing a DCHECK to be hit.

This patch fixes it by making sure that no failable code is run before
acquiring the thread token, thereby ensuring that the thread token is
always acquired and thus avoiding the above crash.

A test is added to confirm this as well. This test crashes without the
code changes in this patch.

Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Reviewed-on: http://gerrit.cloudera.org:8080/11096
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/fragment-instance-state.cc
M tests/failure/test_failpoints.py
2 files changed, 16 insertions(+), 2 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-08-01 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..


Patch Set 3: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 02 Aug 2018 02:20:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-08-01 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2900/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 01 Aug 2018 23:01:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-08-01 Thread Sailesh Mukil (Code Review) 
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..


Patch Set 3: Code-Review+2

(1 comment)

Carry +2.

http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:

http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py@147
PS2, Line 147: query_options={'debug_action':debug_action})
> Should we also verify that the query actually failed ?
It is already doing that since the function called is:
execute_query_expect_failure()



--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 01 Aug 2018 23:01:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-08-01 Thread Michael Ho (Code Review) 
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:

http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py@147
PS2, Line 147: query_options={'debug_action':debug_action})
Should we also verify that the query actually failed ?



--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 01 Aug 2018 22:46:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-07-31 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11096 )

Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/122/ : 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/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Wed, 01 Aug 2018 00:03:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank

2018-07-31 Thread Sailesh Mukil (Code Review) 
Sailesh Mukil has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11096


Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to 
initialize the filter bank
..

IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter 
bank

While Prepare()-ing a fragment instance, if we fail to initialize the
runtime filter bank, we will exit FIS::Prepare() without acquiring a
thread token (AcquireThreadToken()):

FIS::Finalize() is called always regardless of whether the fragment
instance succeeded or failed. And FIS::Finalize() tries to ReleaseThreadToken()
even though it might not have gotten acquired, causing a DCHECK to be hit.

This patch fixes it by making sure that no failable code is run before
acquiring the thread token, thereby ensuring that the thread token is
always acquired and thus avoiding the above crash.

A test is added to confirm this as well. This test crashes without the
code changes in this patch.

Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
---
M be/src/runtime/fragment-instance-state.cc
M tests/failure/test_failpoints.py
2 files changed, 16 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil