[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 12: Thanks for all of your reviews! -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 12 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 10 May 2024 02:08:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add BE tests in network-util-test.cc - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py, test_query_live.py, and test_query_log.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Reviewed-on: http://gerrit.cloudera.org:8080/21383 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/service/query-state-record.cc M be/src/util/network-util-test.cc M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py M tests/util/workload_management.py 14 files changed, 303 insertions(+), 37 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 12 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 10 May 2024 02:08:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:59:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10629/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 11 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:59:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 10: Code-Review+2 Carry +2 since change in ps10 is small. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:58:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16158/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:55:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 10: Code-Review+1 looks good! -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:50:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/9/be/src/util/network-util.cc File be/src/util/network-util.cc: http://gerrit.cloudera.org:8080/#/c/21383/9/be/src/util/network-util.cc@243 PS9, Line 243: if (address.has_uds_address()) t_address.__set_uds_address(address.uds_address()); > Found one issue after this change: Done -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:32:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#10). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add BE tests in network-util-test.cc - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py, test_query_live.py, and test_query_log.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/service/query-state-record.cc M be/src/util/network-util-test.cc M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py M tests/util/workload_management.py 14 files changed, 303 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/10 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 10 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 9: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10626/ -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:22:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/9/be/src/util/network-util.cc File be/src/util/network-util.cc: http://gerrit.cloudera.org:8080/#/c/21383/9/be/src/util/network-util.cc@243 PS9, Line 243: if (address.has_uds_address()) t_address.__set_uds_address(address.uds_address()); Found one issue after this change: https://github.com/apache/impala/blob/d1d28c0/be/src/service/query-state-record.cc#L224-L227 For consistency, these lines of code should be replaced with: TNetworkAddress host = FromNetworkAddressPB(b.address()); -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 20:14:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 15:05:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10626/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 15:05:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 15:05:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 8: Code-Review+2 Upgrading to +2 as others already gave +1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 15:04:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16153/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 14:38:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16152/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 14:35:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/service/data-stream-service.cc@137
PS6, Line 137: few m
> The comment was not update to the new timing
Done
http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/util/network-util.h
File be/src/util/network-util.h:
http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/util/network-util.h@119
PS6, Line 119: if (lhs.has_uds_address()) {
> This is not consistent now with KrpcAddressEqual, as it doesn't consider ha
Updated KrpcAddressEqual to call CompareNetworkAddressPB.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Thu, 09 May 2024 14:14:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#8). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add BE tests in network-util-test.cc - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util-test.cc M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 12 files changed, 297 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/8 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#7). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add BE tests in network-util-test.cc - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util-test.cc M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 12 files changed, 296 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/7 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 7 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 6: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/service/data-stream-service.cc@137 PS6, Line 137: 100ms The comment was not update to the new timing http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/21383/6/be/src/util/network-util.h@119 PS6, Line 119: /// The order is decided first by hostname, then by port, then by uds address. This is not consistent now with KrpcAddressEqual, as it doesn't consider has_uds_address, and considers an empty uds_address equal with a !has_uds_address. This shouldn't matter in practice, but it looks confusing. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Thu, 09 May 2024 11:54:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16145/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 22:11:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 22:06:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/util/network-util.h@118 PS5, Line 118: /// Compare function for two NetworkAddressPB. > Please add unit tests in network-util-test.cc either in this change or a fo Done -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 21:47:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#6). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add BE tests in network-util-test.cc - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util-test.cc M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 12 files changed, 295 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/6 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 6 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Jason Fehr has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 5: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc@159
PS4, Line 159: } else if (qs->is_initialized()) {
> It is more likely, but if IsTerminalState() is true, there is no need to pr
Done
http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/util/network-util.h
File be/src/util/network-util.h:
http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/util/network-util.h@118
PS5, Line 118: /// Compare function for two NetworkAddressPB.
Please add unit tests in network-util-test.cc either in this change or a
follow-up change.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 08 May 2024 20:46:30 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h@328 PS5, Line 328: BackendExecState exec_state = backend_exec_state_; > backend_exec_state_ can only transition to terminal state once and wont rev Ack -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 20:41:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h@328 PS5, Line 328: BackendExecState exec_state = backend_exec_state_; > Shouldn't this be atomic if we need to worry about it's value changing? Or backend_exec_state_ can only transition to terminal state once and wont revert back. This is just to ensure that we are comparing 1 constant value for 3 times below rather than checking against backend_exec_state_ that might be modified in between three checks (say, backend_exec_state_lock_ is changing from EXECUTING to FINISHED when in the middle of backend_exec_state_ == ERROR check). I don't think obtaining backend_exec_state_lock_ is necessary because it will release the lock anyway when applying runtime filter update. Existing codes in query-state.cc also call IsTerminalState() without holding lock. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 20:35:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16141/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 20:34:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/21383/5/be/src/runtime/query-state.h@328 PS5, Line 328: BackendExecState exec_state = backend_exec_state_; Shouldn't this be atomic if we need to worry about it's value changing? Or take backend_exec_state_lock_? -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 20:21:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc@341
PS4, Line 341: if (UNLIKELY(FLAGS_sort_runtime_filter_aggregator_candidates))
{
> Would it be worthwhile to wrap this condition with UNLIKELY since the flag
Done
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc@159
PS4, Line 159: } else if (qs->is_initialized()) {
> Is this condition the more likely to happen? If so, please make it the if
It is more likely, but if IsTerminalState() is true, there is no need to
process filter update at all.
Therefore, it is checked first in the earlier branch.
Removed IsCancelled() check since IsTerminalState() already include
BackendExecState::CANCELLED checking.
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h
File be/src/util/network-util.h:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h@124
PS4, Line 124: if (lhs.has_uds_address()) {
> What if only one has_uds_address? Seems like they should be unequal then.
Done.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 08 May 2024 20:17:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#5). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 10 files changed, 190 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/5 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h@124 PS4, Line 124: comp = lhs.uds_address().compare(rhs.uds_address()); What if only one has_uds_address? Seems like they should be unequal then. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 18:19:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Jason Fehr has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 4: Code-Review+1
(2 comments)
Looks good, couple of questions that may not need code changes depending on the
answer.
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc@341
PS4, Line 341: if (FLAGS_sort_runtime_filter_aggregator_candidates) {
Would it be worthwhile to wrap this condition with UNLIKELY since the flag is
intended for testing?
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc@159
PS4, Line 159: } else if (qs->is_initialized()) {
Is this condition the more likely to happen? If so, please make it the if
condition and the existing if condition becomes the else if:
if (qs->is_initialized()) {
...
} else if (qs.get() == nullptr || qs->IsCancelled() || qs->IsTerminalState()) {
...
}
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jason Fehr
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 08 May 2024 18:18:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16135/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 15:47:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc@59
PS3, Line 59: DEFINE_bool_hidden(sort_runtime_filter_aggregator_candidates,
false,
> candidates
Done
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc@59
PS3, Line 59: sort_runtime_filter_aggregator_candidate
> typo: _candidates
Done
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc@143
PS3, Line 143: sleep_duration_ms
> Maybe even less than 20ms. Especially if these sleeps can stack with comple
Changed to begin with 2ms sleep, and then keep doubling for next iteration.
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/util/network-util.h
File be/src/util/network-util.h:
http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/util/network-util.h@123
PS3, Line 123: if (comp == 0 && lhs.has_uds_address() &&
rhs.has_uds_address()) {
> Check has_uds_address since it is optional field.
Done
http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py
File tests/custom_cluster/test_runtime_filter_aggregation.py:
http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py@89
PS3, Line 89: cls._init_delay]
> nit: +2 indentations on broken lines
Done
http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py@121
PS3, Line 121: all_blocked = 'UpdateFilterFromRemote RPC called with
remaining wait time'
> Could we check in the profile that the runtime filter did not arrive in the
Done
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 08 May 2024 15:28:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Kurt Deschler, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#4). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms, with exponential sleep period in between. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 10 files changed, 180 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/4 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc@59 PS3, Line 59: DEFINE_bool_hidden(sort_runtime_filter_aggregator_cadidates, false, candidates http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc@143 PS3, Line 143: sleep_duration_ms > Isn't this too much, especially at the beginning? I would vote a short slee Maybe even less than 20ms. Especially if these sleeps can stack with complex plan. http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/util/network-util.h@123 PS3, Line 123: if (comp == 0) comp = lhs.uds_address().compare(rhs.uds_address()); Check has_uds_address since it is optional field. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 14:45:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/scheduling/scheduler.cc@59 PS3, Line 59: sort_runtime_filter_aggregator_cadidates typo: _candidates http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/3/be/src/service/data-stream-service.cc@143 PS3, Line 143: sleep_duration_ms Isn't this too much, especially at the beginning? I would vote a short sleep, e.g. 20ms on the first sleep. http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py File tests/custom_cluster/test_runtime_filter_aggregation.py: http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py@89 PS3, Line 89: cls, 'debug_action', nit: +2 indentations on broken lines http://gerrit.cloudera.org:8080/#/c/21383/3/tests/custom_cluster/test_runtime_filter_aggregation.py@121 PS3, Line 121: all_blocked = 'UpdateFilterFromRemote RPC called with remaining wait time' Could we check in the profile that the runtime filter did not arrive in the end? -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 08 May 2024 14:33:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16124/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 22:17:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 22:09:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc@141 PS2, Line 141: bool query_found = false; > nit: could be useful as a hidden config option. Done http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py File tests/custom_cluster/test_runtime_filter_aggregation.py: http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py@115 PS2, Line 115: #The JOIN BUILD fragment in first and third impalads immediately receive EOS > typo: anc -> and Done -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 21:53:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#3). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 10 files changed, 171 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/3 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 2: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/2/be/src/service/data-stream-service.cc@141 PS2, Line 141: int32_t min_wait_time_ms = 500; nit: could be useful as a hidden config option. http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py File tests/custom_cluster/test_runtime_filter_aggregation.py: http://gerrit.cloudera.org:8080/#/c/21383/2/tests/custom_cluster/test_runtime_filter_aggregation.py@115 PS2, Line 115: #The JOIN BUILD fragment in first anc third impalads immediately receive EOS typo: anc -> and -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 21:25:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16091/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Fri, 03 May 2024 01:09:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85:
RETURN_IF_ERROR(DebugAction(query_ctx.client_request.query_options.debug_action,
> Maybe we need to add a query option to fix those for testing so we can have
Changed the test and debug action in ps2.
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@126
PS1, Line 126: const UpdateFilterParamsPB* req, UpdateFilterResultPB* resp,
RpcContext* context) {
> It share the same RPC queue with TransmitData (KRPC exchanges). There is a
I figured the downside of this approach is that RPC thread will blocked until
filter update finally applied or wait period pass. I fix wait period to 500ms
to not hold RPC thread for too long.
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@166
PS1, Line 166: } while (total_wait_time < min_wait_time_ms);
> Ack. "no longer running" sound better.
Done
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py
File tests/query_test/test_runtime_filters.py:
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py@555
PS1, Line 555:
> The longer SLEEP debug action should log error. I'll see if I can add that.
Done
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Fri, 03 May 2024 00:49:16 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Hello Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21383 to look at the new patch set (#2). Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The wait time is fixed at 500ms. If wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add test_runtime_filter_aggregation.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/scheduling/scheduler.cc M be/src/service/data-stream-service.cc M be/src/util/network-util.h M common/protobuf/data_stream_service.proto M tests/custom_cluster/test_runtime_filter_aggregation.py 10 files changed, 169 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/2 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85: {FLAGS_krpc_port == 27000 ? "coordinator" : "executor"}));
> This test is tricky because:
Maybe we need to add a query option to fix those for testing so we can have
deterministic testing?
We probably also want tests that are indeterministic since the feature itself
is, and have probabilistic coverage. So test needs to be flexible enough to
handle different scenarios.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Thu, 02 May 2024 16:10:34 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85: {FLAGS_krpc_port == 27000 ? "coordinator" : "executor"}));
> This seems like an odd way to check when is_coordinator and is_executor exi
This test is tricky because:
- No fragment instances nor RuntimeFilterBank active until QueryState pass
initialization.
- In 3 nodes impala minicluster, all of them are is_coordinator and
is_executor. Only our py.test code enforce that query execution of EE test go
to the first impalad, that is the one with krpc_port=27000
- The test scenario ideally just want to delay the randomly-selected
intermediate aggregator node, but we won't be able to determine if this impalad
is it or not until QueryState is initialized. That is why SLEEP is injected for
all but first impalad.
- The test rely on JOIN BUILD assigned in first impalad (27000) to send
UpdateFilterFromRemote to either the second or the third that is selected as
filter aggregator. This is only possible if this DebugAction does not trigger
in the first impalad.
But even so, I still find undeterminism happen due to data partitioning
schedule. That test scenario that I mention about only work if the build
scanner fragment and join build fragment both colocated at the first impalad
and build scanner does not exchange to the other two impalads. So far, I have
not found any way to make scheduling and exchange direction more deterministic.
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@126
PS1, Line 126: const UpdateFilterParamsPB* req, UpdateFilterResultPB* resp,
RpcContext* context) {
> What thread does this run in? Do we need to worry about it blocking a queue
It share the same RPC queue with TransmitData (KRPC exchanges). There is a
possibility of blocked queue, just as it does with TransmitData.
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@166
PS1, Line 166: PrintId(ProtoToQueryId(req->query_id())), query_found ?
"has done" : "not found",
> I'm not clear what "Query State for query_id=... has done after N ms" would
Ack. "no longer running" sound better.
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py
File tests/query_test/test_runtime_filters.py:
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py@555
PS1, Line 555: """Test that distributed runtime filter aggregation still
works
> Can we also test for the log message? assert_impalad_log_contains would wor
The longer SLEEP debug action should log error. I'll see if I can add that.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 01 May 2024 23:30:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 ) Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16080/ : 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/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Wed, 01 May 2024 23:13:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/runtime/query-exec-mgr.cc@85
PS1, Line 85: {FLAGS_krpc_port == 27000 ? "coordinator" : "executor"}));
This seems like an odd way to check when is_coordinator and is_executor exist.
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc
File be/src/service/data-stream-service.cc:
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@126
PS1, Line 126: const UpdateFilterParamsPB* req, UpdateFilterResultPB* resp,
RpcContext* context) {
What thread does this run in? Do we need to worry about it blocking a queue?
http://gerrit.cloudera.org:8080/#/c/21383/1/be/src/service/data-stream-service.cc@166
PS1, Line 166: PrintId(ProtoToQueryId(req->query_id())), query_found ?
"has done" : "not found",
I'm not clear what "Query State for query_id=... has done after N ms" would
mean.
I guess "done" is a state? Usually not used in that tense, so maybe "no longer
running".
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py
File tests/query_test/test_runtime_filters.py:
http://gerrit.cloudera.org:8080/#/c/21383/1/tests/query_test/test_runtime_filters.py@555
PS1, Line 555: """Test that distributed runtime filter aggregation still
works
Can we also test for the log message? assert_impalad_log_contains would work.
Looks like it might need a separate test case with different parameters.
--
To view, visit http://gerrit.cloudera.org:8080/21383
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9
Gerrit-Change-Number: 21383
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Wed, 01 May 2024 23:05:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21383 Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote .. IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote It is possible to have UpdateFilterFromRemote RPC arrive to an impalad executor before QueryState of the destination query is created or complete initialization. This patch add wait mechanism in UpdateFilterFromRemote RPC endpoint to wait for few miliseconds until QueryState exist and complete initialization. The maximum wait time is max between 200ms and the remaining runtime filter wait time from caller's perspective. If maximum wait time passed and QueryState still not found or initialized, UpdateFilterFromRemote RPC is deemed fail and query execution move on without complete filter. Testing: - Add test_runtime_filters.py::TestLateQueryStateInit - Pass exhastive runs of test_runtime_filters.py and test_runtime_filter_aggregation.py Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 --- M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-filter-bank.cc M be/src/runtime/runtime-filter-bank.h M be/src/service/data-stream-service.cc M common/protobuf/data_stream_service.proto M tests/query_test/test_runtime_filters.py 8 files changed, 120 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21383/1 -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 1 Gerrit-Owner: Riza Suminto
