You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2023/12/15 01:21:27 UTC
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20799
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
IMPALA-12633: Restore checking inflight in retry
Restores checking that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight.
Adds a debug action and test that just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/impala-server.cc
M tests/custom_cluster/test_query_retries.py
3 files changed, 30 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14729/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 01:51:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Dec 2023 15:29:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc@1488
PS7, Line 1488: LOG(WARNING) << "Query " << PrintId(query_id) << " is already in-flight.";
> I'm going to also add a DCHECK, since this shouldn't happen.
May want to put explicit error message for the DCHECK(false) there.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 21:03:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10088/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 04:54:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 12: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 22:37:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10082/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 19:09:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#4).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
4 files changed, 66 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/4
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@375
PS3, Line 375: It's possible (but rare)
: // that the query is not marked inflight yet; what happens in that case is:
> Make sense.
I did all the rest. Avoiding extraneous INFO messages seems worth a little extra bookkeeping.
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@380
PS3, Line 380: 3.
> Will +1 once this nit addressed.
Done
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 22:13:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4:
> Patch Set 4: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10047/
This hit IMPALA-12630. Rebase to pick up fix.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Dec 2023 04:24:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#3).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/impala-server.cc
M tests/custom_cluster/test_query_retries.py
3 files changed, 47 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/3
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@375
PS3, Line 375: It's possible (but rare)
: // that the query is not marked inflight yet; what happens in that case is:
> Question: this looks like a fair number of concurrency scenario. Is it poss
I'm not sure we can make it more deterministic; that has a lot of potential side-effects.
We could track queries that are closed before they're inflight prevent adding them to inflight_queries. That adds a little overhead, but would avoid the extra logging. I'm not sure it's worth warning in that case; there's nothing a user can do about it.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 21:30:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#12).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Adds an option to CloseClientRequestState for when we close a
ClientRequestState but the query is still in-flight with a new
ClientRequestState, like for retries. It ensures that we bypass most of
SetQueryInflight in case CloseClientRequestState was called first.
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 104 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/12
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Code-Review+2
LGTM. Carry Riza's +1.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 04:52:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10079/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 16:59:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 10: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10070/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 02:15:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#14).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Adds an option to CloseClientRequestState for when we close a
ClientRequestState but the query is retrying with a new state. It
ensures that we bypass most of SetQueryInflight in case
CloseClientRequestState was called first.
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 99 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/14
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#9).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 94 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/9
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/2/tests/custom_cluster/test_query_retries.py
File tests/custom_cluster/test_query_retries.py:
http://gerrit.cloudera.org:8080/#/c/20799/2/tests/custom_cluster/test_query_retries.py@777
PS2, Line 777: 'debug_action': 'SET_QUERY_INFLIGHT:SLEEP@1000'})
Ok, after fixing the check the coordinator crashes due to
client-request-state.cc:179] Check failed: wait_thread_.get() == __null BlockOnWait() needs to be called!
Which makes sense to me, it hasn't successfully closed.
I don't see a reason to skip closing the request state if it hasn't been set to inflight yet.
The only consequence I can identify is that the original query might be added to inflight_queries, then not removed until the session is closed. But once the overall query is closed, both the original and retrying query move to completed queries. And despite not being removed from inflight_queries, the query is removed from query_driver_map_ so it's not observable in the web UI. The only consequence a user might observe is a failure trying to UnregisterQuery when the session is closed.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 20:20:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10047/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Dec 2023 03:16:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10070/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 21:26:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc@376
PS1, Line 376: // If the query's not yet inflight, skip it. It should be unregistered later.
> Should this be
Oh, yes it should. I went through several iterations of trying different things and messed it up at the end. But this is part of my concern that "I can't get this section to matter" as well.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 18:15:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14787/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 20:43:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 11: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10079/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 18:38:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10050/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Dec 2023 05:06:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc@376
PS1, Line 376: // If the query's not yet inflight, skip it. It should be unregistered later.
This makes me uneasy. I don't see a reason we would want to skip cleanup because a query hadn't yet been registered. But given I haven't been able to find any specific issue with it I'm erring on restoring the prior behavior rather than removing the check.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 01:25:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc@1615
PS4, Line 1615: query_handle->session()->prestopped_queries.insert(query_handle->query_id());
I don't think we'll ever call CloseClientRequestState twice. That would mess with metrics and other things if we did. If that did happen, we might also add an entry to prestopped_queries after it was previously erased from inflight_queries, which seems pretty harmless.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 22:18:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10043/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 18:15:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14786/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:57:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc@1488
PS7, Line 1488: LOG(WARNING) << "Query " << PrintId(query_id) << " is already in-flight.";
I'm going to also add a DCHECK, since this shouldn't happen.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 20:10:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc@376
PS1, Line 376: // If the query's not yet inflight, skip it. It should be unregistered later.
> This makes me uneasy. I don't see a reason we would want to skip cleanup be
Should this be
if (!query_handle->is_inflight()) return;
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 04:20:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Dec 2023 09:41:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@375
PS3, Line 375: It's possible (but rare)
: // that the query is not marked inflight yet; what happens in that case is:
> I'm not sure we can make it more deterministic; that has a lot of potential
Make sense.
IMPALA-12493 did this change:
- RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr));
+ DCHECK(query_handle->is_inflight());
+ query_handle->Finalize(nullptr);
parent_server_->CloseClientRequestState(query_handle);
parent_server_->MarkSessionInactive(session);
Finalize() changed to void, so it is not returning ERROR from Cancel() anymore.
CloseClientRequestState() look safe to call in any case, (inflight_queries.erase will just turn no op).
MarkSessionInactive() is also valid to call since the refcount decrease match refcount increase from MarkSessionActive() in line 266. This looks OK.
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@380
PS3, Line 380: 1.
> nit: 3
Will +1 once this nit addressed.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 21:58:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#2).
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
IMPALA-12633: Restore checking inflight in retry
Restores checking that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight.
Adds a debug action and test that just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/impala-server.cc
M tests/custom_cluster/test_query_retries.py
3 files changed, 30 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/2
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/11/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/11/be/src/service/impala-server.cc@1631
PS11, Line 1631: << " closed before in-flight.";
There seems to be another scenario besides retries where this can happen. Usually associated with errors earlier in the execution process via HS2.
I'm going to limit this check to CloseClientRequestState called from retries, because that's the only case where we still except SetQueryInflight to be called.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 18:28:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#7).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 92 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/7
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10087/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Dec 2023 18:32:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has removed a vote on this change.
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14788/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 21:51:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc@179
PS5, Line 179: BlockOnWait()
This error message is stale and misleading. This line was added in commit bf8e1b8. At that time, BlockOnWait() calls wait_thread.reset(). But now wait_thread_ is only reset in Finalized(). I think we should change this to "Finalized()".
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h@764
PS5, Line 764: /// in-flight until the query is unregistered. Until a query is in-flight, an attempt
It'd be nice to also mention prestopped_queries.
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc@1615
PS4, Line 1615: query_handle->session()->prestopped_queries.insert(query_handle->query_id());
> I don't think we'll ever call CloseClientRequestState twice. That would mes
We can check whether the insertion took place and add a warning log if it doesn't. The return value of set::insert() can be used.
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc@1616
PS5, Line 1616: ++query_handle->session()->total_queries;
Could you add a comment here for why we bump the counter when closing a query?
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 11:23:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14796/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 19:35:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 12: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10082/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Dec 2023 23:38:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/12/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/20799/12/be/src/service/impala-server.h@802
PS12, Line 802: /// Set 'early' if closing a ClientRequestState while the query is still in-progress.
We could infer this from IsSetRetriedId.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Dec 2023 17:14:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14805/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Dec 2023 18:50:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10088/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 09:29:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@375
PS3, Line 375: It's possible (but rare)
: // that the query is not marked inflight yet; what happens in that case is:
Question: this looks like a fair number of concurrency scenario. Is it possible to identify the code path and put WARNING log for each case? Or make it more deterministic to eliminate some possible scenario?
http://gerrit.cloudera.org:8080/#/c/20799/3/be/src/runtime/query-driver.cc@380
PS3, Line 380: 1.
nit: 3
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 20:48:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10047/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 22:48:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Restore checking inflight in retry
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Restore checking inflight in retry
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14745/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 18:43:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:
http://gerrit.cloudera.org:8080/#/c/20799/1/be/src/runtime/query-driver.cc@376
PS1, Line 376: // that the query is not marked inflight yet; what happens in that case is:
> Oh, yes it should. I went through several iterations of trying different th
Done
http://gerrit.cloudera.org:8080/#/c/20799/2/tests/custom_cluster/test_query_retries.py
File tests/custom_cluster/test_query_retries.py:
http://gerrit.cloudera.org:8080/#/c/20799/2/tests/custom_cluster/test_query_retries.py@777
PS2, Line 777: 'debug_action': 'SET_QUERY_INFLIGHT:SLEEP@1000'})
> Ok, after fixing the check the coordinator crashes due to
Done
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 20:34:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Dec 2023 23:13:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Code-Review+2
Carry +1 from Riza
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 04:47:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#10).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 95 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/10
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/client-request-state.cc@179
PS5, Line 179: Finalize() ne
> This error message is stale and misleading. This line was added in commit b
Done
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h
File be/src/service/impala-server.h:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.h@764
PS5, Line 764: /// in-flight until the query is unregistered. Until a query is in-flight, an attempt
> It'd be nice to also mention prestopped_queries.
Done
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/4/be/src/service/impala-server.cc@1615
PS4, Line 1615: query_handle->UnRegisterRemainingRPCs();
> We can check whether the insertion took place and add a warning log if it d
Done
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/5/be/src/service/impala-server.cc@1616
PS5, Line 1616:
> Could you add a comment here for why we bump the counter when closing a que
I rearranged SetQueryInflight a little bit to handle incrementing it; conceptually it makes more sense that it's still handled when SetQueryInflight is called.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:35:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Verified+1
I've looked into the new failure, and it appears to be an IOException originating from within the iceberg library. I'll file a ticket.
I had a previous run on patch set 14 that succeeded, and this failure looks unrelated, so merging.
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 17:23:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 20:15:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Adds an option to CloseClientRequestState for when we close a
ClientRequestState but the query is retrying with a new state. It
ensures that we bypass most of SetQueryInflight in case
CloseClientRequestState was called first.
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Reviewed-on: http://gerrit.cloudera.org:8080/20799
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 99 insertions(+), 11 deletions(-)
Approvals:
Riza Suminto: Looks good to me, but someone else must approve
Wenzhe Zhou: Looks good to me, approved
Quanlong Huang: Looks good to me, approved
Michael Smith: Verified
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 15
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14751/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 22:40:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14750/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 20:56:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14785/ : 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/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 19:57:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10043/
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 23:21:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20799
to look at the new patch set (#6).
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Removes the DCHECK that the original query is inflight before trying to
close it during a query retry. SetQueryInflight is a separate operation
the server performs after a query has started executing async, and it's
possible for the query to fail and retry before the server calls
SetQueryInflight. When that happens, we still need to perform cleanup
or the original request_state is never closed and we hit a different
DCHECK: "BlockOnWait() needs to be called!"
Updates the message from DCHECK in ClientRequestState's destructor to
reflect that wait_thread_ is only reset in Finalize.
Adds a debug action and test where just the original query is delayed
during the SetQueryInflight call.
Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
---
M be/src/runtime/query-driver.cc
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M tests/custom_cluster/test_query_retries.py
5 files changed, 91 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/20799/6
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 10: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/20799/7/be/src/service/impala-server.cc@1488
PS7, Line 1488: LOG(WARNING) << "Query " << PrintId(query_id) << " is already in-flight.";
> May want to put explicit error message for the DCHECK(false) there.
Done
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Dec 2023 21:25:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 4: Code-Review+1
Thanks for adding prestopped_queries!
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Dec 2023 22:16:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has removed a vote on this change.
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12633: Remove DCHECK for slow SetQueryInflight
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20799 )
Change subject: IMPALA-12633: Remove DCHECK for slow SetQueryInflight
......................................................................
Patch Set 14: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20799
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic17a5e12d9db61cb19306270174518a8dfd281a7
Gerrit-Change-Number: 20799
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Dec 2023 02:37:18 +0000
Gerrit-HasComments: No