Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/15378 )
Change subject: IMPALA-9466: impala-shell client retry for hs2-http protocol ...................................................................... Patch Set 6: (14 comments) Thanks for the review. Please see my response inlined. I am going to incorporate your suggestion next. http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@657 PS5, Line 657: > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@704 PS5, Line 704: r > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@718 PS5, Line 718: f > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@737 PS5, Line 737: I > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@749 PS5, Line 749: r > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@780 PS5, Line 780: l > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@820 PS5, Line 820: > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@838 PS5, Line 838: r > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@852 PS5, Line 852: r > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@861 PS5, Line 861: d > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@871 PS5, Line 871: d > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@881 PS5, Line 881: > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/5/shell/impala_client.py@905 PS5, Line 905: > flake8: E306 expected 1 blank line before a nested definition, found 0 Done http://gerrit.cloudera.org:8080/#/c/15378/6/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/15378/6/shell/impala_client.py@933 PS6, Line 933: _do_hs2_rpc > if the goal is to recover from connections that are closed by the server, i I think thats a valid point. The goal really is to retry in case of failure, when possible. We could have rpcs return 5xx errors (which doesn't mean connection drop) and those should be retried. We could also have something even more drastic in which case the connection drops (something I have seen recently). I think `self.transport.isOpen()` should tell us whether the transport is open or not. -- To view, visit http://gerrit.cloudera.org:8080/15378 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0da9e9e8d34a340eaf763397cc095ff6260d65d5 Gerrit-Change-Number: 15378 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Comment-Date: Thu, 12 Mar 2020 01:49:06 +0000 Gerrit-HasComments: Yes
