[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 3: Verified+1 -- 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: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 15 May 2019 10:58:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY This patch removes the FAULT_INJECTION_RPC_DELAY macro and replaces its uses with DebugAction which is more flexible. For example, it supports JITTER which injects random delays. Every backend rpc has a debug action of the form RPC_NAME_DELAY. DebugAction has previously always been used via query options. However, for the rpcs considered here there is not always a query with an accessible TQUeryOptions available (for example, we do not send any query info with the RemoteShutdown rpc), so this patch introduces a flag, '--debug_actions', which is used to control these rpc delay debug actions. Testing: - Updated existing tests to use the new mechanism. Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Reviewed-on: http://gerrit.cloudera.org:8080/13060 Reviewed-by: Thomas Marshall Tested-by: Impala Public Jenkins --- M be/src/common/global-flags.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/fault-injection-util.cc M be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M tests/custom_cluster/test_rpc_timeout.py 9 files changed, 54 insertions(+), 83 deletions(-) Approvals: Thomas Marshall: Looks good to me, approved Impala Public Jenkins: Verified -- 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: merged Gerrit-Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Gerrit-Change-Number: 13060 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4260/ DRY_RUN=false -- 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: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 15 May 2019 05:43:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3234/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 15 May 2019 05:20:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
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 3: Code-Review+2 carrying forward -- 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: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 15 May 2019 04:25:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Hello Michael Ho, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13060 to look at the new patch set (#3). Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY This patch removes the FAULT_INJECTION_RPC_DELAY macro and replaces its uses with DebugAction which is more flexible. For example, it supports JITTER which injects random delays. Every backend rpc has a debug action of the form RPC_NAME_DELAY. DebugAction has previously always been used via query options. However, for the rpcs considered here there is not always a query with an accessible TQUeryOptions available (for example, we do not send any query info with the RemoteShutdown rpc), so this patch introduces a flag, '--debug_actions', which is used to control these rpc delay debug actions. Testing: - Updated existing tests to use the new mechanism. Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b --- M be/src/common/global-flags.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/fault-injection-util.cc M be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M tests/custom_cluster/test_rpc_timeout.py 9 files changed, 54 insertions(+), 83 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/13060/3 -- 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: newpatchset Gerrit-Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Gerrit-Change-Number: 13060 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 2: Code-Review+2 (1 comment) 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."); > I could see the argument either way. I think this is fine as-is. No point in dwelling on it for too much as long as it achieves the purpose. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 18 Apr 2019 22:26:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2835/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Thu, 18 Apr 2019 18:35:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
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
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Thu, 18 Apr 2019 17:59:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Hello Michael Ho, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13060 to look at the new patch set (#2). Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY This patch removes the FAULT_INJECTION_RPC_DELAY macro and replaces its uses with DebugAction which is more flexible. For example, it supports JITTER which injects random delays. Every backend rpc has a debug action of the form RPC_NAME_DELAY. DebugAction has previously always been used via query options. However, for the rpcs considered here there is not always a query with an accessible TQUeryOptions available (for example, we do not send any query info with the RemoteShutdown rpc), so this patch introduces a flag, '--debug_actions', which is used to control these rpc delay debug actions. Testing: - Updated existing tests to use the new mechanism. Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b --- M be/src/common/global-flags.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/fault-injection-util.cc M be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M tests/custom_cluster/test_rpc_timeout.py 9 files changed, 54 insertions(+), 83 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/13060/2 -- 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: newpatchset Gerrit-Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Gerrit-Change-Number: 13060 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2826/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Wed, 17 Apr 2019 21:55:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13060 ) Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. Patch Set 1: (1 comment) 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 -- 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: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Wed, 17 Apr 2019 21:14:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Remove FAULT INJECTION RPC DELAY
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13060 Change subject: IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY .. IMPALA-8138: Remove FAULT_INJECTION_RPC_DELAY This patch removes the FAULT_INJECTION_RPC_DELAY macro and replaces its uses with DebugAction which is more flexible. For example, it supports JITTER which injects random delays. Every backend rpc has a debug action of the form RPC_NAME_DELAY. DebugAction has previously always been used via query options. However, for the rpcs considered here there is not always a query with an accessible TQUeryOptions available (for example, we do not send any query info with the RemoteShutdown rpc), so this patch introduces a flag, '--debug_actions', which is used to control these rpc delay debug actions. Testing: - Updated existing tests to use the new mechanism. Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b --- M be/src/common/global-flags.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/fault-injection-util.cc M be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M tests/custom_cluster/test_rpc_timeout.py 9 files changed, 52 insertions(+), 81 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/60/13060/1 -- 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: newchange Gerrit-Change-Id: I712b188e0cdf91f431c9b94052501e5411af407b Gerrit-Change-Number: 13060 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
