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 <[email protected]> Gerrit-Reviewer: Anuj Phadke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Wed, 03 Oct 2018 21:24:33 +0000 Gerrit-HasComments: Yes
