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 3: (4 comments) Thanks for the review. http://gerrit.cloudera.org:8080/#/c/15378/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15378/3//COMMIT_MSG@18 PS3, Line 18: atleast > Nit: at least Done. http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@645 PS3, Line 645: self.max_tries = 3 > Design question -- is it better to a single max_tries value that belongs to Adding "max_tries" parameter to _do_hs2_rpc() probably makes sense to allow for future changes in that direction. I chose 3, since it is the magic retry number every seems to be using. The tricky part is to figure out the max tries. One option is that we could make that a configurable option with default value of 3 and range [1,5]. http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@657 PS3, Line 657: r > I'm not sure it makes a huge difference either way, but these do seem equiv Changed to using def instead. http://gerrit.cloudera.org:8080/#/c/15378/3/shell/impala_client.py@674 PS3, Line 674: raise_error = num_tries == max_tries > Nit: minor style consideration. Done. -- 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: 3 Gerrit-Owner: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Comment-Date: Tue, 10 Mar 2020 22:26:22 +0000 Gerrit-HasComments: Yes
