Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 )
Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/common/global-flags.cc@154 PS1, Line 154: DEFINE_string(debug_actions, "", "For testing only. Uses the same format as the debug " : "action query options, but allows for injection of debug actions in code paths where " : "query options are not available."); > If set, should this also affect the default value of debug action query opt I could see the argument either way. I chose to do it this way because this way the flag and the query option are used for disjoint sets of labels - it seems like it would be more confusing to me if some debug actions can be set either with the flag or the query option but others can only be set with the flag. But admittedly its confusing this way as well. Another option I considered was naming the flag something like "rpc_debug_actions", though I decided against it in case we want to use this functionality to add more non-rpc debug actions without query options in the future. Of course, if that happens we could just deal with it then. Maybe we can come up with a name to the effect of "non_query_specific_debug_actions" but less clunky, like "system_debug_actions"? http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/service/data-stream-service.cc@97 PS1, Line 97: EndDataStreamResponsePB* response, RpcContext* rpc_context) { > May wanna add a delay here too ? Done http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/util/debug-util.h File be/src/util/debug-util.h: http://gerrit.cloudera.org:8080/#/c/13060/1/be/src/util/debug-util.h@144 PS1, Line 144: Status DebugActionImpl(const string& debug_action, const char* label) WARN_UNUSED_RESULT; > or FLAGS_debug_action Done http://gerrit.cloudera.org:8080/#/c/13060/1/tests/custom_cluster/test_rpc_timeout.py File tests/custom_cluster/test_rpc_timeout.py: http://gerrit.cloudera.org:8080/#/c/13060/1/tests/custom_cluster/test_rpc_timeout.py@129 PS1, Line 129: > flake8: E502 the backslash is redundant between brackets Done -- To view, visit http://gerrit.cloudera.org:8080/13060 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Gerrit-Change-Number: 13060 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Thu, 18 Apr 2019 17:59:24 +0000 Gerrit-HasComments: Yes
