You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sailesh Mukil (Code Review)" <ge...@cloudera.org> on 2017/04/07 05:43:26 UTC
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Sailesh Mukil has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6587
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
IMPALA-5182: Explicitly close connection to impalad on error from shell
When using connections secured with SSL, a connection close comprises
of a bi-directional SSL_shutdown(). The second part of the
bi-directional shutdown requires that the client also close the socket
explicitly, and the server blocks till it gets the close
notification from the client.
This patch ensures that the above happens. Without this fix, the
impala-shell was found to hang over connections secured with SSL
when an error was encountered.
Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
---
M shell/impala_shell.py
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/6587/1
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 2:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/441/
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 1:
(1 comment)
> (1 comment)
>
> Were you able to repro the issue? Do you think it's easy/feasible
> to add a test?
Yes, I was able to repro it, but only on clusters with LDAP enabled.
I found this issue by looking at the thread stacks during the hang. The repro steps are:
- Connect with "impala-shell -l -u <user> --ssl"
- Give an incorrect LDAP password
- Open another terminal and try to open impala-shell, and it hangs (until you kill the impala-shell on the first terminal)
When you give an incorrect LDAP password, you get the following error in the shell:
"Error connecting: TTransportException, TSocket read 0 bytes"
However, when you try repro-ing with only SSL (and no LDAP), i.e. use a wrong CA cert from the impala-shell, we get the following error:
"Error connecting: TTransportException, Could not connect to localhost:21000"
and the above hang is not reproducible.
In the LDAP case, it connects to the impalad using SSL and then requests for the LDAP password. In the only SSL case, it's not even able to connect to the impalad and therefore the hang doesn't happen.
So since we unfortunately don't have LDAP for our unit testing, we can't include a test for this.
http://gerrit.cloudera.org:8080/#/c/6587/1/shell/impala_shell.py
File shell/impala_shell.py:
Line 708: # Secure connections may still be open. So we explicitly close it.
> connection (singular)
Done
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 3:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/487/
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 2: Code-Review+2
Thanks for the explanation. Change looks simple enough for me to +2.
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 1:
(1 comment)
Were you able to repro the issue? Do you think it's easy/feasible to add a test?
http://gerrit.cloudera.org:8080/#/c/6587/1/shell/impala_shell.py
File shell/impala_shell.py:
Line 708: # Secure connections may still be open. So we explicitly close it.
connection (singular)
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has uploaded a new patch set (#2).
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
IMPALA-5182: Explicitly close connection to impalad on error from shell
When using connections secured with SSL, a connection close comprises
of a bi-directional SSL_shutdown(). The second part of the
bi-directional shutdown requires that the client also close the socket
explicitly, and the server blocks till it gets the close
notification from the client.
This patch ensures that the above happens. Without this fix, the
impala-shell was found to hang over connections secured with SSL
when an error was encountered.
Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
---
M shell/impala_shell.py
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/6587/2
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
Patch Set 3: Code-Review+2
Rebased. Carry +2.
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5182: Explicitly close connection to impalad on error from shell
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5182: Explicitly close connection to impalad on error from shell
......................................................................
IMPALA-5182: Explicitly close connection to impalad on error from shell
When using connections secured with SSL, a connection close comprises
of a bi-directional SSL_shutdown(). The second part of the
bi-directional shutdown requires that the client also close the socket
explicitly, and the server blocks till it gets the close
notification from the client.
This patch ensures that the above happens. Without this fix, the
impala-shell was found to hang over connections secured with SSL
when an error was encountered.
Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Reviewed-on: http://gerrit.cloudera.org:8080/6587
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M shell/impala_shell.py
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Sailesh Mukil: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/6587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I814df93bbcd457ad3f96b4c1ef5d8b0ddd6d141f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>