impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5558/IMPALA-5576: Reopen stale client connection
Date Mon, 26 Jun 2017 03:26:41 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5558/IMPALA-5576: Reopen stale client connection
......................................................................


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7284/11/be/src/runtime/backend-client.h
File be/src/runtime/backend-client.h:

Line 54:     // Cannot inject fault on recv() side as the callers cannot handle it.
Why not? Should we leave a todo here?


http://gerrit.cloudera.org:8080/#/c/7284/10/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

PS10, Line 348: . This is benign as cancellation is racy.
Can you change this to explain what exactly the race is? I.e. "cannot be found in the backend
because the fragment instance may have been cancelled by reason-xyz in the meantime".


http://gerrit.cloudera.org:8080/#/c/7284/10/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

Line 139:     /// Used to handle duplicate done ReportExecStatus RPC messages.
mention "in ApplyExecStatusReport" to emphasize that this is not used elsewhere?


http://gerrit.cloudera.org:8080/#/c/7284/11/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:

Line 230:     ImpalaBackendConnection client(ExecEnv::GetInstance()->impalad_client_cache(),
Could this return 3 broken clients in a row? Why is it necessary to get a new client inside
the loop?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d722c8ad3bf0e78e89887b6cb286c69ca61b8f5
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Juan Yu <jyu@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message