Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/12260 )
Change subject: IMPALA-7985: Port RemoteShutdown() to KRPC. ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/12260/2/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/12260/2/be/src/service/client-request-state.cc@692 PS2, Line 692: if (msg.find("RemoteShutdown() RPC failed: Timed out: connection negotiation to") : != string::npos) { : // Prior to IMPALA-7985 :shutdown() used the backend port. : err_string.append(" This may be because the port specified is a thrift port, which" : " :shutdown() can no longer use. Please make sure the right port" : " is being used, or don't specify any port in the :shutdown()" : " command."); > Not sure if a typical user has any understanding of the difference between Done http://gerrit.cloudera.org:8080/#/c/12260/3/be/src/service/control-service.cc File be/src/service/control-service.cc: http://gerrit.cloudera.org:8080/#/c/12260/3/be/src/service/control-service.cc@170 PS3, Line 170: FAULT_INJECTION_RPC_DELAY(RPC_CANCELQUERYFINSTANCES); > Shouldn't this use a DebugAction instead ? These tests are setting delays on server-side commands so as to test RPC timeouts. They FAULT_INJECTION_RPC_DELAY injections are configured by running servers with --fault_injection_rpc_type=XXX. DebugAction on the other hand is configured for a particular query by passing --query_option=YYY to a client like impala-shell. So in this case FAULT_INJECTION_RPC_DELAY seems the right choice. http://gerrit.cloudera.org:8080/#/c/12260/3/be/src/service/control-service.cc@184 PS3, Line 184: FAULT_INJECTION_RPC_DELAY(RPC_REMOTESHUTDOWN); > Shouldn't this use a DebugAction instead ? These tests are setting delays on server-side commands so as to test RPC timeouts. They FAULT_INJECTION_RPC_DELAY injections are configured by running servers with --fault_injection_rpc_type=XXX. DebugAction on the other hand is configured for a particular query by passing --query_option=YYY to a client like impala-shell. So in this case FAULT_INJECTION_RPC_DELAY seems the right choice. http://gerrit.cloudera.org:8080/#/c/12260/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12260/2/be/src/service/impala-server.cc@2489 PS2, Line 2489: SleepForMs(1000) > It is an official C++ feature to extend the life time of a temporary object Thanks, done -- To view, visit http://gerrit.cloudera.org:8080/12260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4fd00ee4e638f5e71e27893162fd65501ef9e74e Gerrit-Change-Number: 12260 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Sherman <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Wed, 30 Jan 2019 20:01:40 +0000 Gerrit-HasComments: Yes
