[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12142 ) Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1700/ : 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/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 03 Jan 2019 01:35:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Andrew Sherman has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12142 ) Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. IMPALA-7468: Port CancelQueryFInstances() to KRPC. When the Coordinator needs to cancel a query (usually because a user has hit Control-C), it does this by sending a CancelQueryFInstances message to each fragment instance. This change switches this code to use KRPC. Add new protobuf definitions for the messages, and remove the old thrift definitions. Move the server-side implementation of Cancel() from ImpalaInternalService to ControlService. Rework the scheduler so that the FInstanceExecParams always contains the KRPC address of the fragment executors, this address can then be used if a query is to be cancelled. For now keep the KRPC calls to CancelQueryFInstances() as synchronous. While moving the client-side code, remove the fault injection code that was inserted with FAULT_INJECTION_SEND_RPC_EXCEPTION and FAULT_INJECTION_RECV_RPC_EXCEPTION (triggered by running impalad with --fault_injection_rpc_exception_type=1) as this tickles code in client-cache.h which is now not used. TESTING: Ran all end-to-end tests. No new tests as test_cancellation.py provides good coverage. Checked in debugger that DebugAction style fault injection (triggered from test_cancellation.py) was working correctly. Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 --- M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/control-service.cc M be/src/service/control-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M common/protobuf/control_service.proto M common/thrift/ImpalaInternalService.thrift 12 files changed, 158 insertions(+), 111 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/12142/3 -- To view, visit http://gerrit.cloudera.org:8080/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Andrew Sherman has posted comments on this change. (
http://gerrit.cloudera.org:8080/12142 )
Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC.
..
Patch Set 1:
(6 comments)
Thanks for the helpful review
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/runtime/coordinator-backend-state.h@294
PS1, Line 294: Status DoCancelQueryFInstancesRrpcWithRetry(
> Nit: The prevailing style seems to place reference ('&') and pointer ('*')
Up to now I have just been using what the clang-format rules dictate. In
clang-format terms I think you are saying we should use
'DerivePointerAlignment: true' (which means: "analyze the formatted file for
the most common alignment of & and *. Pointer and reference alignment styles
are going to be updated according to the preferences found in the file.
PointerAlignment is then used only as fallback"), is that the standard practice
for Impala?
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/query-schedule.h@104
PS1, Line 104: const TNetworkAddress& krpc_host, int
per_fragment_instance_idx,
> Nit: I wonder if there are trailing whitespaces in these lines? I don't und
There is no trailing whitespace I could find.
I think this formatting is the result of the clang-format parameter:
ConstructorInitializerAllOnOneLineOrOnePerLine: true
which is splitting the initializers to one-per-line.
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.h@433
PS1, Line 433: void ComputeFragmentExecParams(const BackendConfig
executor_config,
> Nit: Please retain existing placement of '&' and '*'.
Thanks for catching my not using a reference for executor_config.
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.cc@333
PS1, Line 333: ComputeFragmentExecParams(executor_config, plan_exec_info,
> Nit: Trailing spaces?
Yes there is something wrong here and I have reformatted
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto
File common/protobuf/control_service.proto:
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto@163
PS1, Line 163: optional UniqueIdPB query_id = 1;
> Don't understand how the query id can be optional...
OK you made me think!
I had just copied what other people had done in implementing krpc. But now I
have investigated further and found that many people argue that having required
fields is a bad idea, and the syntax is even removed in future protobuf
implementations, see for example
https://stackoverflow.com/questions/31801257/why-required-and-optional-is-removed-in-protocol-buffers-3
Overall this makes sense to me, so I think optional is OK here, are you
persuaded?
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto@179
PS1, Line 179: // Cancellation is asynchronous.
> Commit message says cancellation is synchronous. Do you want to change this
When the coordinator cancels a query it sends CancelQueryFInstancesRequestPB
messages to the hosts executing fragment instances for the query. These calls
are synchronous, that is the coordinator waits for a response to each
CancelQueryFInstances call before making the next one. This is what the commit
message means by 'For now keep the KRPC calls to CancelQueryFInstances() as
synchronous'. When a cancellation occurs on an impalad executing a fragment
instance then the cancellation is implemented by setting flags saying the query
is cancelled in various data structures, and by waking various sleeping
threads. This should be enough to stop the query executing, but the
CancelQueryFInstances call returns without doing any checking that all query
execution has stopped. So in this sense cancel is asynchronous. I updated the
comment to make this clearer.
--
To view, visit http://gerrit.cloudera.org:8080/12142
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969
Gerrit-Change-Number: 12142
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Thu, 03 Jan 2019 00:33:30 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12142 ) Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1699/ : 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/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 02 Jan 2019 19:52:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Zoram Thanga has posted comments on this change. (
http://gerrit.cloudera.org:8080/12142 )
Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC.
..
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/runtime/coordinator-backend-state.h@294
PS1, Line 294: Status DoCancelQueryFInstancesRrpcWithRetry(
Nit: The prevailing style seems to place reference ('&') and pointer ('*')
symbols next to the pointed-to-type, rather than next to the variable name.
Comment applies for the entire review.
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/query-schedule.h
File be/src/scheduling/query-schedule.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/query-schedule.h@104
PS1, Line 104: const TNetworkAddress& krpc_host, int
per_fragment_instance_idx,
Nit: I wonder if there are trailing whitespaces in these lines? I don't
understand why some lines are marked as changed...
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.h@433
PS1, Line 433: void ComputeFragmentExecParams(const BackendConfig
executor_config,
Nit: Please retain existing placement of '&' and '*'.
Also: Are you intentionally passing executor_config by value? Here and in the
following functions?
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/scheduling/scheduler.cc@333
PS1, Line 333: ComputeFragmentExecParams(executor_config, plan_exec_info,
Nit: Trailing spaces?
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto
File common/protobuf/control_service.proto:
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto@163
PS1, Line 163: optional UniqueIdPB query_id = 1;
Don't understand how the query id can be optional...
http://gerrit.cloudera.org:8080/#/c/12142/1/common/protobuf/control_service.proto@179
PS1, Line 179: // Cancellation is asynchronous.
Commit message says cancellation is synchronous. Do you want to change this
comment until async cancellation is implemented?
--
To view, visit http://gerrit.cloudera.org:8080/12142
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969
Gerrit-Change-Number: 12142
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Wed, 02 Jan 2019 19:29:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Andrew Sherman has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12142 ) Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. IMPALA-7468: Port CancelQueryFInstances() to KRPC. When the Coordinator needs to cancel a query (usually because a user has hit Control-C), it does this by sending a CancelQueryFInstances message to each fragment instance. This change switches this code to use KRPC. Add new protobuf definitions for the messages, and remove the old thrift definitions. Move the server-side implementation of Cancel() from ImpalaInternalService to ControlService. Rework the scheduler so that the FInstanceExecParams always contains the KRPC address of the fragment executors, this address can then be used if a query is to be cancelled. For now keep the KRPC calls to CancelQueryFInstances() as synchronous. While moving the client-side code, remove the fault injection code that was inserted with FAULT_INJECTION_SEND_RPC_EXCEPTION and FAULT_INJECTION_RECV_RPC_EXCEPTION (triggered by running impalad with --fault_injection_rpc_exception_type=1) as this tickles code in client-cache.h which is now not used. TESTING: Ran all end-to-end tests. No new tests as test_cancellation.py provides good coverage. Checked in debugger that DebugAction style fault injection (triggered from test_cancellation.py) was working correctly. Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 --- M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/control-service.cc M be/src/service/control-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M common/protobuf/control_service.proto M common/thrift/ImpalaInternalService.thrift 12 files changed, 164 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/12142/2 -- To view, visit http://gerrit.cloudera.org:8080/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12142 ) Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1698/ : 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/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 02 Jan 2019 18:50:30 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11517 ) Change subject: [WIP] IMPALA-6932: Speed up scans for sequence datasets with many files .. Patch Set 3: > Patch Set 3: > > The discussion looks like it's not easy to add the tests on HDFS. Should we > try S3 then? Hi Lars, I have tried testing on S3. I did notice a reduction in the query time. I was querying the tpch_avro.lineitem table and the timeline difference was 8.66s vs 5.87s. The scanner I/O wait time went down from 2.371s to 0.851s. However, I couldn't notice a deterministic counter which can be verified each time the test is run. Tim suggested that I could add a counter which keeps track of the number of scan ranges read for each scan node. I was thinking I'll add that in a separate patch and then test it here. Do you have any other suggestions? Thanks, Pooja -- To view, visit http://gerrit.cloudera.org:8080/11517 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I211e2511ea3bb5edea29f1bd63e6b1fa4c4b1965 Gerrit-Change-Number: 11517 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 02 Jan 2019 18:44:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12024 ) Change subject: IMPALA-7625: test_web_pages.py backend tests are failing .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1697/ : 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/12024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb Gerrit-Change-Number: 12024 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 02 Jan 2019 18:40:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7666: Adding an opaque client identifier to query options.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12130 ) Change subject: IMPALA-7666: Adding an opaque client identifier to query options. .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12130/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12130/2//COMMIT_MSG@15 PS2, Line 15: protoype *prototype Are you going to include that in this patch? That would be cool. Alternatively it might be nice to include a proof-of-concept in impala-shell too. http://gerrit.cloudera.org:8080/#/c/12130/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12130/2/be/src/service/query-options.cc@724 PS2, Line 724: .c_str() Can't we just pass the original string? -- To view, visit http://gerrit.cloudera.org:8080/12130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a7708492f05d33b2bc99fc3a03b461bbb6f3ea4 Gerrit-Change-Number: 12130 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 02 Jan 2019 18:39:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7970 : Add support for metastore event based automatic invalidate
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12118 ) Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1696/ : 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/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 12 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 02 Jan 2019 18:29:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/12142 )
Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC.
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/service/control-service.cc
File be/src/service/control-service.cc:
http://gerrit.cloudera.org:8080/#/c/12142/1/be/src/service/control-service.cc@166
PS1, Line 166: class CancelQueryFInstancesResponsePB* response,
::kudu::rpc::RpcContext* rpc_context) {
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12142
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969
Gerrit-Change-Number: 12142
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 02 Jan 2019 18:21:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7468: Port CancelQueryFInstances() to KRPC.
Andrew Sherman has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12142 Change subject: IMPALA-7468: Port CancelQueryFInstances() to KRPC. .. IMPALA-7468: Port CancelQueryFInstances() to KRPC. When the Coordinator needs to cancel a query (usually because a user has hit Control-C), it does this by sending a CancelQueryFInstances message to each fragment instance. This change switches this code to use KRPC. Add new protobuf definitions for the messages, and remove the old thrift definitions. Move the server-side implementation of Cancel() from ImpalaInternalService to ControlService. Rework the scheduler so that the FInstanceExecParams always contains the KRPC address of the fragment executors, this address can then be used if a query is to be cancelled. For now keep the KRPC calls to CancelQueryFInstances() as synchronous. While moving the client-side code, remove the fault injection code that was inserted with FAULT_INJECTION_SEND_RPC_EXCEPTION and FAULT_INJECTION_RECV_RPC_EXCEPTION (triggered by running impalad with --fault_injection_rpc_exception_type=1) as this tickles code in client-cache.h which is now not used. TESTING: Ran all end-to-end tests. No new tests as test_cancellation.py provides good coverage. Checked in debugger that DebugAction style fault injection (triggered from test_cancellation.py) was working correctly. Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 --- M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/scheduling/query-schedule.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/control-service.cc M be/src/service/control-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M common/protobuf/control_service.proto M common/thrift/ImpalaInternalService.thrift 12 files changed, 165 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/12142/1 -- To view, visit http://gerrit.cloudera.org:8080/12142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I625030c3f1068061aa029e6e242f016cadd84969 Gerrit-Change-Number: 12142 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman
[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/12024 ) Change subject: IMPALA-7625: test_web_pages.py backend tests are failing .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/12024/4/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/12024/4/tests/common/impala_test_suite.py@837 PS4, Line 837: while (actual_state != expected_state and time.time() - start_time < timeout): > nit: you don't need parentheses around the while condition Done http://gerrit.cloudera.org:8080/#/c/12024/4/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/12024/4/tests/webserver/test_web_pages.py@271 PS4, Line 271: cancels the query.""" > Should we mention that this function now optionally waits for a particular Done -- To view, visit http://gerrit.cloudera.org:8080/12024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb Gerrit-Change-Number: 12024 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 02 Jan 2019 18:06:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing
Hello Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12024 to look at the new patch set (#5). Change subject: IMPALA-7625: test_web_pages.py backend tests are failing .. IMPALA-7625: test_web_pages.py backend tests are failing The backend tests in test_web_page.py do some basic validation of the query_backends and query_finstances endpoints. The tests were failing due to a race condition in the test itself. The issue is that the backend endpoints are only usable while a query is running. When queried against completed queries, the backend endpoints return nothing. The original test worked by running a "complex" query asynchronously and then querying the backend endpoints before the query completed. The assumption was that the query would run long enough for the request to the backend endpoint to complete. However, that is not always the case and the query easily completes before the backend endpoints can be reached. This patch updates the test so that instead of running a "complex" query, it runs a query that calls the sleep UDF, guaranteeing the query will take enough time for the backend endpoints to be reached. Furthermore, the patch adds additional validation and cleanup of all the backend endpoint tests. Testing: Ran core tests. Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb --- M tests/common/impala_test_suite.py M tests/custom_cluster/test_admission_controller.py M tests/webserver/test_web_pages.py 3 files changed, 83 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/12024/5 -- To view, visit http://gerrit.cloudera.org:8080/12024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb Gerrit-Change-Number: 12024 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-7970 : Add support for metastore event based automatic invalidate
Vihang Karajgaonkar has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/12118 ) Change subject: IMPALA-7970 : Add support for metastore event based automatic invalidate .. IMPALA-7970 : Add support for metastore event based automatic invalidate This change adds support to CatalogD to poll metastore events to issue invalidate on tables automatically. It adds basic infrastructure to poll HMS notifications events at a configurable frequency using a backend config called hms_event_polling_frequency_s flag. Currently, it issues invalidate at tables when it received alter events on table and partitions. It also adds tables/databases and removes tables from catalogD when it receives create_table/create_database and drop_table/drop_database events. The default value of hms_event_polling_frequency_s is 0 which disables the feature. A non-zero value in seconds of this configuration can be used to enable the feature and set the polling frequency. In order to process each event atomically, this feature relies on version lock in CatalogServiceCatalog. It adds new methods in CatalogServiceCatalog which takes a write lock on version so that readers are blocked until the catalog state is updated based on the events. In case of processing events, the metastore operation is already completed and only catalog state needs to be updated. Hence we do not need to make new metastore calls while processing the events and only version lock is sufficient to serialize updates to the catalog objects based on events. This locking protocol is similar to what is done in case of DDL processing in CatalogOpExecutor except it does not need to take metastoreDdlLock since no metastore operations are needed during event processing. The change also adds a new test class to test the basic functionality for each of the event type which is supported. Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java A fe/src/main/java/org/apache/impala/catalog/MetastoreEventHandlerUtils.java A fe/src/main/java/org/apache/impala/catalog/MetastoreEventsProcessor.java A fe/src/main/java/org/apache/impala/catalog/MetastoreNotificationException.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java A fe/src/test/java/org/apache/impala/catalog/MetastoreEventsProcessorTest.java M fe/src/test/resources/postgresql-hive-site.xml.template 10 files changed, 1,367 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12118/12 -- To view, visit http://gerrit.cloudera.org:8080/12118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic70b27780560b7ac9b33418d132b36cd0ca4abf7 Gerrit-Change-Number: 12118 Gerrit-PatchSet: 12 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment ids.
Zoram Thanga has posted comments on this change. (
http://gerrit.cloudera.org:8080/12129 )
Change subject: IMPALA-6664: Tag log statements with fragment ids.
..
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12129/1/be/src/common/logging.cc
File be/src/common/logging.cc:
http://gerrit.cloudera.org:8080/#/c/12129/1/be/src/common/logging.cc@55
PS1, Line 55: void PrependFragment(std::string* s, bool* changed) {
Nice. I think the next step would be to (re)define VLOG_QUERY or something like
it, that would be called for debug logging in the query/FINST lifecycle code?
--
To view, visit http://gerrit.cloudera.org:8080/12129
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8
Gerrit-Change-Number: 12129
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Wed, 02 Jan 2019 17:52:32 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6664: Tag log statements with fragment ids.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12129 ) Change subject: IMPALA-6664: Tag log statements with fragment ids. .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6634ef9d1a7346339f24f2d40a7a3aa36a535da8 Gerrit-Change-Number: 12129 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 02 Jan 2019 14:28:55 + Gerrit-HasComments: No
