You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Marshall (Code Review)" <ge...@cloudera.org> on 2018/09/18 20:21:38 UTC

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11465


Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................

IMPALA-7488: Fix hang in test_cancellation

test_cancellation runs a impala-shell process with a query specified
then sends a SIGINT and confirms that the shell cancels the query and
exits.

The hang was happening because the shell's signal handler was
incorrectly using the same Thirft connection when calling Close() as
the main shell thread, which is not thread safe.

Testing:
- Ran test_cancellation in a loop 500 times. Previously the hang would
  repro about every 10 runs.

Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
---
M shell/impala_shell.py
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................

IMPALA-7488: Fix hang in test_cancellation

test_cancellation runs a impala-shell process with a query specified
then sends a SIGINT and confirms that the shell cancels the query and
exits.

The hang was happening because the shell's signal handler was
incorrectly using the same Thirft connection when calling Close() as
the main shell thread, which is not thread safe.

Testing:
- Ran test_cancellation in a loop 500 times. Previously the hang would
  repro about every 10 runs.

Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Reviewed-on: http://gerrit.cloudera.org:8080/11465
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/impala_shell.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified
  Tim Armstrong: Looks good to me, but someone else must approve

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Sep 2018 22:56:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Sep 2018 20:55:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3173/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 00:31:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 1: Code-Review+2

Nice find!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Sep 2018 22:34:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 10:03:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2:

The build failure seems to be unrelated, so I have restarted the job. Good catch btw.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 10:06:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Sep 2018 22:56:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

wow

http://gerrit.cloudera.org:8080/#/c/11465/2/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11465/2/shell/impala_shell.py@a536
PS2, Line 536: 
:(



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Sep 2018 23:15:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 )

Change subject: IMPALA-7488: Fix hang in test_cancellation
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2
Gerrit-Change-Number: 11465
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Sep 2018 13:24:04 +0000
Gerrit-HasComments: No