[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-18 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 2: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 13:54:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-18 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..

IMPALA-10704: Fix retried query id not being unregistered when retry fails

When query retry fails in RetryQueryFromThread(), the retried query id
may not be unregistered if the failure happens before we store the
retry_request_state. In this case, QueryDriver::Unregister() has no way
to get the retried query id so it's not deleted. Note that the retried
query id is registered in RetryQueryFromThread() so should be deleted
later. This finally results in a leak in the query driver map, where
queries in it are shown as in-flight queries.

test_retry_query_result_cacheing_failed and
test_retry_query_set_query_in_flight_failed (added in IMPALA-10413)
asserts one in-flight query at the end. This is satisfied by the leak.
Instead, we should verify no running queries at the end.

This patch adds a new field in QueryDriver to remember the registered
retry query id as a backup way for getting it when query retry fails
before we store the ClientRequestState of the retried query (so
retried_client_request_state_ is null).

Tests:
 - Run test_retry_query_result_cacheing_failed and
   test_retry_query_set_query_in_flight_failed 100 times.

Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Reviewed-on: http://gerrit.cloudera.org:8080/17465
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/query-driver.cc
M be/src/runtime/query-driver.h
M tests/custom_cluster/test_query_retries.py
3 files changed, 36 insertions(+), 2 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-18 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 07:57:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-18 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7152/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 07:57:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Xianqing He (Code Review) 
Xianqing He has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 06:39:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Gabor Kaszab (Code Review) 
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 06:37:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Wenzhe Zhou (Code Review) 
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h
File be/src/runtime/query-driver.h:

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h@265
PS1, Line 265: registered_retry_query_id_
> I think we don't need this because the default constructor of TUniqueId alr
Got it. Thanks



--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 01:24:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Quanlong Huang (Code Review) 
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h
File be/src/runtime/query-driver.h:

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h@265
PS1, Line 265: registered_retry_query_id_
> Should we set the initial value zero for this variable explicitly?
I think we don't need this because the default constructor of TUniqueId already 
does it: be/generated-sources/gen-cpp/Types_types.h

class TUniqueId {
 public:

  TUniqueId(const TUniqueId&);
  TUniqueId(TUniqueId&&);
  TUniqueId& operator=(const TUniqueId&);
  TUniqueId& operator=(TUniqueId&&);
  TUniqueId() : hi(0), lo(0) {// <--- The default is setting hi and 
lo to 0
  }
};



--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Tue, 18 May 2021 01:02:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Wenzhe Zhou (Code Review) 
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1: -Code-Review

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h
File be/src/runtime/query-driver.h:

http://gerrit.cloudera.org:8080/#/c/17465/1/be/src/runtime/query-driver.h@265
PS1, Line 265: registered_retry_query_id_
Should we set the initial value zero for this variable explicitly?



--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Mon, 17 May 2021 16:45:27 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Wenzhe Zhou (Code Review) 
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Mon, 17 May 2021 16:38:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17465 )

Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/8727/ : 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/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Reviewer: Xianqing He 
Gerrit-Comment-Date: Mon, 17 May 2021 10:30:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10704: Fix retried query id not being unregistered when retry fails

2021-05-17 Thread Quanlong Huang (Code Review) 
Quanlong Huang has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/17465


Change subject: IMPALA-10704: Fix retried query id not being unregistered when 
retry fails
..

IMPALA-10704: Fix retried query id not being unregistered when retry fails

When query retry fails in RetryQueryFromThread(), the retried query id
may not be unregistered if the failure happens before we store the
retry_request_state. In this case, QueryDriver::Unregister() has no way
to get the retried query id so it's not deleted. Note that the retried
query id is registered in RetryQueryFromThread() so should be deleted
later. This finally results in a leak in the query driver map, where
queries in it are shown as in-flight queries.

test_retry_query_result_cacheing_failed and
test_retry_query_set_query_in_flight_failed (added in IMPALA-10413)
asserts one in-flight query at the end. This is satisfied by the leak.
Instead, we should verify no running queries at the end.

This patch adds a new field in QueryDriver to remember the registered
retry query id as a backup way for getting it when query retry fails
before we store the ClientRequestState of the retried query (so
retried_client_request_state_ is null).

Tests:
 - Run test_retry_query_result_cacheing_failed and
   test_retry_query_set_query_in_flight_failed 100 times.

Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
---
M be/src/runtime/query-driver.cc
M be/src/runtime/query-driver.h
M tests/custom_cluster/test_query_retries.py
3 files changed, 36 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/17465/1
--
To view, visit http://gerrit.cloudera.org:8080/17465
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I074526799d68041a425b2379e74f8d8b45ce892a
Gerrit-Change-Number: 17465
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang