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

[Impala-ASF-CR] [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

anujphadke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11540


Change subject: [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect
......................................................................

[WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 33 insertions(+), 9 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke <ap...@cloudera.com>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................

IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 5 sec.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 5 sec.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 52 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 9
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
Hello Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 38 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 7
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 10 Oct 2018 22:05:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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/11540 )

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................

IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 5 sec.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell (with ssl enabled) connects to this socket and
times out after 2 sec.

2. Created a kerberized impala cluster with ssl enabled and
connected to the impalad using an openssl client (block the
beeswax server thread to accept new connection) -

E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later.
impala-shell timed out after the default of 5 sec.I verified
it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Reviewed-on: http://gerrit.cloudera.org:8080/11540
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 52 insertions(+), 15 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 12
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 5
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 04 Oct 2018 00:14:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 41 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 5
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 9:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@265
PS8, Line 265:     if self.verbose:
> This message seems misleading, given that the client has its own notion of 
Done


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@266
PS8, Line 266:       print_to_stderr('Opened TCP connection to %s:%s' % (self.impalad_host,
> "on sockets."
Done


http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py@816
PS6, Line 816:       raise
> I think we should keep this, see my comment elsewhere
Done


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1510
PS8, Line 1510:   """Replaces variable within the string with their corresponding values using the
> nit: indent
Done


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1540
PS8, Line 1540: def get_var_name(name):
> While you're here, "Looks", "Returns"
Done


http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py@737
PS6, Line 737:        but doing this reliably on different Linux distributions is quite hard.
> Where did this go?
This test hung forever after I moved the unsetting of timeout before the ping RPC.
Checked with Lars and we figured out that we can still recreate this by invoking the impala-shell with the SSL flag.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 9
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 06:40:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................

IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 40 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 6
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@261
PS1, Line 261:     self.connected = True
> Does not work as expected if we pass 0
This explains why: https://docs.python.org/2/library/socket.html#socket.socket.settimeout

Please add a comment to explain that passing None is required to set the socket to blocking.


http://gerrit.cloudera.org:8080/#/c/11540/4/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/shell/impala_client.py@280
PS4, Line 280:        This function returns the transport object and its associated socket.
Please keep the order consistent between the name, the comment, and the returned tuple, i.e. socket, transport.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py@54
PS1, Line 54:             'client_connect_timeout_ms': 5000,
> I tried to keep this value large by default for secure connections to estab
Some stress testing in a larger cluster under load would be good. See the comments in IMPALA-7638 for more context.


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
> I suppose one way to disable the timeout is to have a very large timeout va
I'm in favor of sticking to 0 to disable it, which is in line with what we do elsewhere. It'll help prevent confusion for users, should they want to disable it and try to set it to 0.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 03 Oct 2018 21:24:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

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

Change subject: [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@226
PS1, Line 226:  
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@744
PS1, Line 744: ,
flake8: E231 missing whitespace after ','


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@747
PS1, Line 747: r
flake8: E501 line too long (94 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@748
PS1, Line 748: r
flake8: F841 local variable 'results' is assigned to but never used



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 18:45:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Oct 2018 08:48:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 6
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 10 Oct 2018 20:54:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 9
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 06:06:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
> I'm in favor of sticking to 0 to disable it, which is in line with what we 
That's also a fine option. It seems the current behavior with 0 is not well defined anyway so may as well special case it to disable the timeout switch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 03 Oct 2018 21:28:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
> I tried setting it here
I suppose one way to disable the timeout is to have a very large timeout value. So, in that sense, it's probably okay to not have a value to disable the timeout.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 03 Oct 2018 21:18:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................

IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 5 sec.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 5 sec.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
4 files changed, 32 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 8
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@265
PS8, Line 265:       print_to_stderr('Connected to %s:%s' % (self.impalad_host, self.impalad_port))
I am moving this here. Logging this  before ping_impala_service() 
should help us narrow down a hang in open() vs ping_impala_service() potentially.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 8
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 16 Oct 2018 22:19:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@61
PS8, Line 61: def print_to_stderr(message):
flake8: E302 expected 2 blank lines, found 1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 8
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 04:28:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py@81
PS3, Line 81: int(client_connect_timeout_ms)
> Any idea what happens if the user passes a negative value ? Also, it seems 
Nope it does not work.
./bin/impala-shell.sh -i localhost:1234 -t -1 -q "show tables"
Starting Impala Shell without Kerberos authentication
Error connecting: ValueError, Timeout value out of range
Not connected to Impala, could not execute queries.
anuj@anuj-OptiPlex-9020:~/Impala$

setTimeout converts it to float by dividing the ms by 1000.0
I am ok with converting it to int here and it getting implicitly typecasted to a float


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py@a864
PS3, Line 864: 
> Why was this removed ?
Moved it back.


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
> Do you think it makes sense to have a value (e.g. 0) which allows user to d
I tried setting it here
1.
 anuj@anuj-OptiPlex-9020:~/Impala$ time ./bin/impala-shell.sh -i localhost:1234 -t 0 -q "show tables"
Starting Impala Shell without Kerberos authentication
Error connecting: TTransportException, Could not connect to localhost:1234
Not connected to Impala, could not execute queries.

real	0m0.121s
user	0m0.084s
sys	0m0.028s
anuj@anuj-OptiPlex-9020:~/Impala$ 

2. 
I tried reverting it back to 0 (in connect())  and that resulted in some of the tests failing.


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@227
PS3, Line 227:  i
> nit: missing space before "if"
Done


http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@740
PS3, Line 740:  
> nit: extra whitespace
Done


http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@742
PS3, Line 742: indefinitely
> indefinitely ?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 4
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 02 Oct 2018 20:50:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@7
PS1, Line 7: meout 
> Rephrase to state what the change does, not what it should do, e.g. "Set so
Done


http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@19
PS1, Line 19: 2. Created a kerberized impala cluster with ssl enabled and connected
> This should be automated, too.
1. I am not sure if it's easy to automate this test on a minicluster setup (kerberos + SSL)
2. I ran  test_client_ssl.py and created a minicluster setup with SSL enabled.  Tried to block the beeswax server with telnet  or with an openssl client,  but it did not block the beeswax server (with just SSL enabled.)


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@81
PS1, Line 81: client_connect_timeout
> Is this milliseconds? If so, please rename to client_connect_timeout_ms
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@254
PS1, Line 254:     sock, self.transport = self._get_socket_and_transport()
             :     sock.setTimeout(self.client_connect_t
> You can write this in a single line:
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@257
PS1, Line 257:     protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
             :     self.imp_service = ImpalaService.Client(protocol)
             :     result = self.ping_impala_service()
             :     sock.setTimeout(None)
> Which of these 4 lines can time out? Which of the ones that can time out do
Let me get back to you shortly on this one.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@261
PS1, Line 261:     self.connected = True
> This could be more clear if we pass 0 instead
Does not work as expected if we pass 0
Here is the definition in -

build/impala-shell-3.1.0-SNAPSHOT/lib/thrift/transport/TSocket.py


def setTimeout(self, ms):
    if ms is None:
      self.__timeout = None
    else:
      self.__timeout = ms / 1000.0


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@274
PS1, Line 274: 
> update the comment
Updated the comment to mention what this function returns.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@326
PS1, Line 326:       return sock, TSaslClientTransport(sasl_factory, "GSSAPI", sock)
> This will now return a tuple in some cases, and a transport in others. Plea
oops. I missed returning from this inner function. Let me make sure I test this part too.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py@864
PS1, Line 864:         self.imp_client.close_connection()
> Please add a test that calls connect from the shell instead of passing a ho
Working on it. Will send a new patch with this change.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py@54
PS1, Line 54:             'client_connect_timeout_ms': 5000,
> How did you decide on 60s? Why not pick something smaller, e.g. 5s?
I tried to keep this value large by default for secure connections to establish. 
I searched around a bit after reading your comment and I  think 5 secs should suffice for a connection to be established. Do you know of any tests to check if 5secs would be good enough?


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@225
PS1, Line 225: client_connect_timeout
> client_connect_timeout_ms?
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@226
PS1, Line 226: "
> flake8: E251 unexpected spaces around keyword / parameter equals
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@737
PS1, Line 737: est_
> nit: Tests
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@743
PS1, Line 743:     """
> Use contextlib.closing here: https://docs.python.org/2/library/contextlib.h
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@744
PS1, Line 744: n
> flake8: E231 missing whitespace after ','
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@745
PS1, Line 745: (
> Please add a comment what this 1 does?
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@746
PS1, Line 746: aximum 
> test_port? It's not an impalad
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@747
PS1, Line 747: 
> use ' and " consistently in this line
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Oct 2018 08:28:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 7: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 7
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 11 Oct 2018 01:59:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

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

Change subject: [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 19:16:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 11:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 11
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 21:45:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_client.py@81
PS3, Line 81: float(client_connect_timeout_ms)
Any idea what happens if the user passes a negative value ? Also, it seems a bit weird that it's a float. Does int not work ?


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/impala_shell.py@a864
PS3, Line 864: 
Why was this removed ?


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
Do you think it makes sense to have a value (e.g. 0) which allows user to disable the timeout behavior ? Actually, have you confirmed the behavior of setting this to 0 ? Does it timeout all the time ?


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@227
PS3, Line 227: if
nit: missing space before "if"


http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@740
PS3, Line 740:  
nit: extra whitespace


http://gerrit.cloudera.org:8080/#/c/11540/3/tests/shell/test_shell_commandline.py@742
PS3, Line 742: indefinately
indefinitely ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Oct 2018 01:07:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 11
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 21:45:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 4
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 02 Oct 2018 21:13:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 5: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@81
PS5, Line 81:     self.client_connect_timeout_ms = int(client_connect_timeout_ms)
nit: move below use_ldap to follow order or arguments


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py@224
PS5, Line 224: 
nit: no newline, follow surrounding code



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 5
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 09 Oct 2018 03:01:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@265
PS8, Line 265:       print_to_stderr('Connected to %s:%s' % (self.impalad_host, self.impalad_port))
This message seems misleading, given that the client has its own notion of being connected (see is_connected() above).

We can change it to "Opened TCP connection to %s:%s" and only print it when the log level is set to debug. If you want to tell both hangs apart, we should use two separate log messages.


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_client.py@266
PS8, Line 266:     # Setting a timeout of None disables timeouts on socket operations
"on sockets."


http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/6/shell/impala_shell.py@816
PS6, Line 816:     except ImportError:
I think we should keep this, see my comment elsewhere


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1510
PS8, Line 1510:   given set_variables."""
nit: indent


http://gerrit.cloudera.org:8080/#/c/11540/8/shell/impala_shell.py@1540
PS8, Line 1540:   """Look for a namespace:var_name pattern in an option name.
While you're here, "Looks", "Returns"


http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/6/tests/shell/test_shell_commandline.py@737
PS6, Line 737:     """
Where did this go?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 8
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 16 Oct 2018 22:31:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11540/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/shell/option_parser.py@227
PS4, Line 227: connect
connect to Impala server.


http://gerrit.cloudera.org:8080/#/c/11540/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/tests/shell/test_shell_commandline.py@749
PS4, Line 749: '-q "select foo; select bar;" -t 2000 ' + '-i localhost:%d' % (test_port)
nit: no need for '+':

   '-q "select foo; select bar;" -t 2000 -i localhost:%d'



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 4
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 03 Oct 2018 00:43:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

Posted by "anujphadke (Code Review)" <ge...@cloudera.org>.
Hello Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 37 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 6
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 10 Oct 2018 22:05:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 8
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 16 Oct 2018 22:48:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 7:

I added some debugging logging to narrow down the failure. The open() in connect seems to finish very quickly in milliseconds. Majority of the time seems to contributed by PingImpalaService()

E   AssertionError: Cmd --query="create table 
test_kudu_dml_reporting_256dcf63.dml_test (id int primary key, age int null) partition by hash(id) partitions 2 stored as kudu" was expected to succeed: Starting Impala Shell without Kerberos authentication
E   time taken to  connect in open(): 0.000133037567139
E   time with socket error: 5.005163908

I removed the handling of exception in the code and the socket timeout exception was thrown in the PingImpala() function - 
Stacktrace

/data/jenkins/workspace/impala-private-parameterized/repos/Impala/tests/shell/test_shell_commandline.py:137: in test_default_db
    result = run_impala_shell_cmd(args)
shell/util.py:97: in run_impala_shell_cmd
    result.stderr)
E   AssertionError: Cmd -d parquet was expected to succeed: Starting Impala Shell without Kerberos authentication
E   Traceback (most recent call last):
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_shell.py", line 1720, in <module>
E       shell = ImpalaShell(options, query_options)
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_shell.py", line 233, in __init__
E       self.do_connect(options.impalad)
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_shell.py", line 799, in do_connect
E       self._connect()
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_shell.py", line 844, in _connect
E       result = self.imp_client.connect()
E     File "/data0/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_client.py", line 260, in connect
E       result = self.ping_impala_service()
E     File "/data0/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/impala_client.py", line 267, in ping_impala_service
E       return self.imp_service.PingImpalaService()
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/gen-py/ImpalaService/ImpalaService.py", line 229, in PingImpalaService
E       return self.recv_PingImpalaService()
E     File "/data/jenkins/workspace/impala-private-parameterized/repos/Impala/shell/gen-py/ImpalaService/ImpalaService.py", line 240, in recv_PingImpalaService
E       (fname, mtype, rseqid) = iprot.readMessageBegin()
E     File "/data/jenkins/workspace/impala-private-parameterized/Impala-Toolchain/thrift-0.9.3-p4/python/lib64/python2.7/site-packages/thrift/protocol/TBinaryProtocol.py", line 126, in readMessageBegin
E       sz = self.readI32()
E     File "/data/jenkins/workspace/impala-private-parameterized/Impala-Toolchain/thrift-0.9.3-p4/python/lib64/python2.7/site-packages/thrift/protocol/TBinaryProtocol.py", line 206, in readI32
E       buff = self.trans.readAll(4)
E     File "/data/jenkins/workspace/impala-private-parameterized/Impala-Toolchain/thrift-0.9.3-p4/python/lib64/python2.7/site-packages/thrift/transport/TTransport.py", line 58, in readAll
E       chunk = self.read(sz - have)
E     File "/data/jenkins/workspace/impala-private-parameterized/Impala-Toolchain/thrift-0.9.3-p4/python/lib64/python2.7/site-packages/thrift/transport/TTransport.py", line 159, in read
E       self.__rbuf = StringIO(self.__trans.read(max(sz, self.__rbuf_size)))
E     File "/data/jenkins/workspace/impala-private-parameterized/Impala-Toolchain/thrift-0.9.3-p4/python/lib64/python2.7/site-packages/thrift/transport/TSocket.py", line 105, in read
E       buff = self.handle.recv(sz)
E   socket.timeout: timed out

I think the fix would be to move the timeout before the PingImpalaService.
Might have to get rid of the test I added as it will connect and the timeout happens in the PingRPC call. 
I will verify it once on a kerberized setup.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 7
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Mon, 15 Oct 2018 07:35:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 37 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 4
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py@770
PS9, Line 770:       # maximum number of queued connections on this socket is 1.
             :       s.listen(1)
Is this needed?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 9
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 18:27:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 5: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@255
PS5, Line 255: # Setting a timeout of None disables timeouts on socket operations
This comment can be moved to above line 264.


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@258
PS5, Line 258:     else:
             :       sock.setTimeout(None)
Given sock is just returned from self._get_socket_and_transport(), I don't think this line is needed.


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py@227
PS5, Line 227:                     " if it fails to connect to Impala server")
Set to 0 to disable any timeout.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 5
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 09 Oct 2018 01:42:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 11
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Thu, 18 Oct 2018 01:41:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: anujphadke <ap...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Oct 2018 08:32:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11540/2/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/2/shell/impala_client.py@326
PS2, Line 326: S
flake8: F821 undefined name 'SaslClientTransport'


http://gerrit.cloudera.org:8080/#/c/11540/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/2/tests/shell/test_shell_commandline.py@748
PS2, Line 748: t
flake8: F841 local variable 'test_port' is assigned to but never used



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Mon, 01 Oct 2018 08:01:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 7
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 10 Oct 2018 22:05:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11540/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11540/9//COMMIT_MSG@16
PS9, Line 16: 1. Added a test where I create a random listening socket.
> This list should also mention the automatic test added in this change.
Done


http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py@770
PS9, Line 770:       # maximum number of queued connections on this socket is 1.
             :       s.listen(1)
> Is this needed?
listen takes 1 value by default
E   TypeError: listen() takes exactly one argument

Do you want me to choose some other value?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 10
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 20:42:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 10: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/9/tests/shell/test_shell_commandline.py@770
PS9, Line 770:       # maximum number of queued connections on this socket is 1.
             :       s.listen(1)
> listen takes 1 value by default
Thanks for checking, let's keep it as it is.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 10
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 21:44:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

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

Change subject: [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect
......................................................................


Patch Set 1:

(17 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@7
PS1, Line 7: should
Rephrase to state what the change does, not what it should do, e.g. "Set socket timeout in impala-shell"


http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@19
PS1, Line 19: 2. Created a kerberized impala cluster with ssl enabled and connected
This should be automated, too.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@81
PS1, Line 81: client_connect_timeout
Is this milliseconds? If so, please rename to client_connect_timeout_ms


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@254
PS1, Line 254:     ret = self._get_socket_and_transport()
             :     sock, self.transport = ret[0], ret[1]
You can write this in a single line:

    sock, self.transport = self._get_socket_and_transport()


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@257
PS1, Line 257:     self.transport.open()
             :     protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
             :     self.imp_service = ImpalaService.Client(protocol)
             :     result = self.ping_impala_service()
Which of these 4 lines can time out? Which of the ones that can time out do we currently test? There doesn't seem to be a test that connects successfully but times out in the ssl connection setup.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@261
PS1, Line 261:     sock.setTimeout(None)
This could be more clear if we pass 0 instead


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@274
PS1, Line 274:     """Create a Transport.
update the comment


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@326
PS1, Line 326:       return TSaslClientTransport(sasl_factory, "GSSAPI", sock)
This will now return a tuple in some cases, and a transport in others. Please make it consistent so that the function returns the same types in all cases.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py@864
PS1, Line 864:         self.imp_client.close_connection()
Please add a test that calls connect from the shell instead of passing a host during startup


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py@54
PS1, Line 54:             'client_connect_timeout': 60000,
How did you decide on 60s? Why not pick something smaller, e.g. 5s?


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@225
PS1, Line 225: client_connect_timeout
client_connect_timeout_ms?


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@737
PS1, Line 737: Test
nit: Tests


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@743
PS1, Line 743:     s = socket.socket()
Use contextlib.closing here: https://docs.python.org/2/library/contextlib.html#contextlib.closing


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@745
PS1, Line 745: 1
Please add a comment what this 1 does?


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@746
PS1, Line 746: impalad
test_port? It's not an impalad


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@747
PS1, Line 747: r
> flake8: E501 line too long (94 > 90 characters)
use ' and " consistently in this line


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@747
PS1, Line 747: 60000
This means that the test takes 60s to execute, right? Can you pick a smaller value instead, e.g. 2s?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 21:21:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11540/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11540/9//COMMIT_MSG@16
PS9, Line 16: 1. Created a kerberized impala cluster with ssl enabled and connected
This list should also mention the automatic test added in this change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 9
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 17 Oct 2018 18:28:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@261
PS1, Line 261:     protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
> This explains why: https://docs.python.org/2/library/socket.html#socket.soc
Done


http://gerrit.cloudera.org:8080/#/c/11540/4/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/shell/impala_client.py@280
PS4, Line 280:        the kerberized version, a sasl transport is created.
> Please keep the order consistent between the name, the comment, and the ret
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py@54
PS1, Line 54:             'client_connect_timeout_ms': 5000,
> Some stress testing in a larger cluster under load would be good. See the c
Will try that.


http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/3/shell/option_parser.py@225
PS3, Line 225: "-t", "--client_connect_timeout_ms"
> That's also a fine option. It seems the current behavior with 0 is not well
anuj@anuj-OptiPlex-9020:~/Impala$ 
anuj@anuj-OptiPlex-9020:~/Impala$ ./bin/impala-shell.sh -i anuj-timeout-3.gce.cloudera.com:21000 --ssl -q "show tables"
Starting Impala Shell without Kerberos authentication
SSL is enabled. Impala server certificates will NOT be verified (set --ca_cert to change)
Error connecting: TTransportException, Could not connect to anuj-timeout-3.gce.cloudera.com:21000: _ssl.c:495: The handshake operation timed out
Not connected to Impala, could not execute queries.


------Without timeout set to 0 it was stuck.
anuj@anuj-OptiPlex-9020:~/Impala$ ./bin/impala-shell.sh -i anuj-timeout-3.gce.cloudera.com:21000 --ssl -q "show tables" -t 0
Starting Impala Shell without Kerberos authentication
SSL is enabled. Impala server certificates will NOT be verified (set --ca_cert to change)


http://gerrit.cloudera.org:8080/#/c/11540/4/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/shell/option_parser.py@227
PS4, Line 227: connect
> connect to Impala server.
Done


http://gerrit.cloudera.org:8080/#/c/11540/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/4/tests/shell/test_shell_commandline.py@749
PS4, Line 749: '-q "select foo; select bar;" -t 2000 -i localhost:%d' % (test_port)
> nit: no need for '+':
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 5
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 03 Oct 2018 23:46:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@81
PS5, Line 81:     self.fetch_batch_size = 1024
> nit: move below use_ldap to follow order or arguments
Done


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@255
PS5, Line 255: if self.client_connect_timeout_ms > 0:
> This comment can be moved to above line 264.
Done


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/impala_client.py@258
PS5, Line 258:     protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
             :     self.imp_service = Impa
> Given sock is just returned from self._get_socket_and_transport(), I don't 
Done


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py@224
PS5, Line 224:   parser.add_option("-t", "--client_connect_timeout_ms",
> nit: no newline, follow surrounding code
Done


http://gerrit.cloudera.org:8080/#/c/11540/5/shell/option_parser.py@227
PS5, Line 227:                     " timeout.")
> Set to 0 to disable any timeout.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 6
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Wed, 10 Oct 2018 20:27:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7555: Set socket timeout in impala-shell

Posted by "Anuj Phadke (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Marshall, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7555: Set socket timeout in impala-shell
......................................................................

IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 5 sec.
Usage: impala-shell --client_connect_timeout=<value in ms>

Testing:
1. Added a test where I create a random listening socket.
impala-shell (with ssl enabled) connects to this socket and
times out after 2 sec.

2. Created a kerberized impala cluster with ssl enabled and
connected to the impalad using an openssl client (block the
beeswax server thread to accept new connection) -

E.g. - openssl s_client -connect <IP Addr>:21000
Used impala-shell to connect to the same impalad later.
impala-shell timed out after the default of 5 sec.I verified
it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 52 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 10
Gerrit-Owner: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Anuj Phadke <ap...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>