You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jason Fehr (Code Review)" <ge...@cloudera.org> on 2024/09/03 22:40:15 UTC

[Impala-ASF-CR] IMPALA-13350: Fixes TSAN Thread Leak Caused Workload Management Thread

Jason Fehr has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21744


Change subject: IMPALA-13350: Fixes TSAN Thread Leak Caused Workload Management Thread
......................................................................

IMPALA-13350: Fixes TSAN Thread Leak Caused Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h.  This thread runs until a graceful shutdown is
intiated.  The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process.  If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
5 files changed, 41 insertions(+), 12 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#7).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Info level logging was added to indicate which completed queries
queue drain situation occurred - successful or timed out.

A new custom cluster test was added to test the situation where the
completed queries queue drain process times out.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 110 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 5:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/16862/ : Initial code review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 00:55:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Info level logging was added to indicate which completed queries
queue drain situation occurred - successful or timed out.

A new custom cluster test was added to test the situation where the
completed queries queue drain process times out.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Reviewed-on: http://gerrit.cloudera.org:8080/21744
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 108 insertions(+), 17 deletions(-)

Approvals:
  Riza Suminto: Looks good to me, approved
  Michael Smith: Looks good to me, but someone else must approve
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 10
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@118
PS2, Line 118:     completed_queries_shutdown_cv_.wait_for(l,
> The condition variable releases the unique_lock while it's waiting.  It ret
If the wait_for function times out, it returns no matter the result of the predicate function (true or false).


http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc@124
PS5, Line 124:   if (workload_mgmt_thread_state_ == SHUTDOW
> clang-tidy complain here:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 15:23:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 6: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc@124
PS5, Line 124: DCHECK_NE(nullptr, workload_management_threa
> Done
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 15:54:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 16:12:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10927/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 22:13:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 00:05:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 00:22:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 23:39:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#8).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Info level logging was added to indicate which completed queries
queue drain situation occurred - successful or timed out.

A new custom cluster test was added to test the situation where the
completed queries queue drain process times out.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 111 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/8
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10930/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 23:57:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#9).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Info level logging was added to indicate which completed queries
queue drain situation occurred - successful or timed out.

A new custom cluster test was added to test the situation where the
completed queries queue drain process times out.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 108 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/9
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc@134
PS8, Line 134: case NOT_STARTED:
             :       // Thread was never started, joining or detaching will cause error.
             :       break;
> This can be removed?
yes



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 23:56:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@118
PS2, Line 118:     completed_queries_cv_.notify_all();
> If the wait_for function times out, it returns no matter the result of the 
Oh right, I spaced and overlooked that it takes 'l' as an argument.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 18:52:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#4).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h.  This thread runs until a graceful shutdown is
intiated.  The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process.  If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
5 files changed, 41 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/4
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#3).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h.  This thread runs until a graceful shutdown is
intiated.  The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process.  If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
5 files changed, 41 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Sep 2024 23:31:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#6).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Info level logging was added to indicate which completed queries
queue drain situation occurred - successful or timed out.

A new custom cluster test was added to test the situation where the
completed queries queue drain process times out.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 97 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG@10
PS2, Line 10: in impala-server.h.  This thread runs until a graceful shutdown is
nit: unnecessary double space. Same on the next line.


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@118
PS2, Line 118:     completed_queries_shutdown_cv_.wait_for(l,
I'm not quite sure how this works. It holds workload_mgmt_threadstate_mu_ and doesn't release it before signaling. So there's no way another thread could update workload_mgmt_thread_state_ to be SHUTDOWN. So will it always wait the full time?


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@127
PS2, Line 127: workload_management_thread_
> Add DCHECK not nullptr.
Is this ever reached? We should log for both branches.


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@130
PS2, Line 130: workload_management_thread_->Detach();
> Will we miss logging some queries into query log table if this happen?
I agree this would be useful.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Sep 2024 23:01:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 1
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Sep 2024 23:08:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 15:54:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 00:08:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc@134
PS8, Line 134: case SHUTDOWN:
             :       // Safe to join the thread here because the workload managmenent processing loop
             :       // set
> yes
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 02:06:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 20:31:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@127
PS2, Line 127: se NOT_STARTED:
> Done
Done


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@130
PS2, Line 130: se SHUTDOWN:
> Yes.  I added logging of the completed queries queued metric which will giv
Done


http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/5/be/src/service/workload-management.cc@124
PS5, Line 124: DCHECK_NOTNULL(workload_management_thread_);
clang-tidy complain here:
/home/ubuntu/Impala/be/src/service/workload-management.cc:124:18: warning: expression result unused [clang-diagnostic-unused-value]

Maybe switch to DCHECK_NE?

DCHECK_NE(nullptr, workload_management_thread_.get());



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 05:20:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/8/be/src/service/workload-management.cc@134
PS8, Line 134: case NOT_STARTED:
             :       // Thread was never started, joining or detaching will cause error.
             :       break;
This can be removed?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 23:46:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 16:20:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#5).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h. This thread runs until a graceful shutdown is
initiated. The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process. If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
M tests/custom_cluster/test_query_log.py
6 files changed, 97 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/5
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak Caused Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Hello Riza Suminto, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21744

to look at the new patch set (#2).

Change subject: IMPALA-13347: Fixes TSAN Thread Leak Caused Workload Management Thread
......................................................................

IMPALA-13347: Fixes TSAN Thread Leak Caused Workload Management Thread

The workload management processing runs in a separate thread declared
in impala-server.h.  This thread runs until a graceful shutdown is
intiated.  The last step of the Impala coordinator shutdown process
is to drain the completed queries queue to the query log table thus
ensuring completed queries do not get lost.

This thread has to run to completion, but the coordinator shutdown
process never joins that thread. This patch adds the joining of that
thread during the coordinator shutdown process.  If the workload
management shutdown process exceedes the allotted time, the thread is
detached.

Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
---
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/workload-management-init.cc
M be/src/service/workload-management.cc
M be/src/service/workload-management.h
5 files changed, 41 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/21744/2
-- 
To view, visit http://gerrit.cloudera.org:8080/21744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10931/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 04:44:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG@11
PS2, Line 11: intiated
nit: initiated


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h
File be/src/service/workload-management.h:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h@101
PS2, Line 101: workload management
nit: workload management setup


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h@108
PS2, Line 108: finishedor
nit: finished or


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@127
PS2, Line 127: workload_management_thread_
Add DCHECK not nullptr.


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@130
PS2, Line 130: workload_management_thread_->Detach();
Will we miss logging some queries into query log table if this happen?
If so, we should log how many queries we might miss after Detach.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 3
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Sep 2024 22:54:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG@10
PS2, Line 10: in impala-server.h. This thread runs until a graceful shutdown is
> nit: unnecessary double space. Same on the next line.
Done


http://gerrit.cloudera.org:8080/#/c/21744/2//COMMIT_MSG@11
PS2, Line 11: initiate
> nit: initiated
Done


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h
File be/src/service/workload-management.h:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h@101
PS2, Line 101: kload management in
> nit: workload management setup
Done


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.h@108
PS2, Line 108: finished o
> nit: finished or
Done


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@118
PS2, Line 118:     completed_queries_cv_.notify_all();
> I'm not quite sure how this works. It holds workload_mgmt_threadstate_mu_ a
The condition variable releases the unique_lock while it's waiting.  It retakes the unique_lock before the predicate function runs and releases it after the predicate completes.

Once the wait finishes, it takes the unique_lock before return ing from the wait_for() function.

Thus, the workload_mgmt_thread_state_ can be updated only while the condition variable is waiting.


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@127
PS2, Line 127: se NOT_STARTED:
> Add DCHECK not nullptr.
Done


http://gerrit.cloudera.org:8080/#/c/21744/2/be/src/service/workload-management.cc@130
PS2, Line 130: se SHUTDOWN:
> Will we miss logging some queries into query log table if this happen?
Yes.  I added logging of the completed queries queued metric which will give a maximum number of queries lost.  I don't want to check the in-memory queue size because taking its lock may cause a hang at the code that timed out could still be running and holding the lock.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 00:23:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2024 23:57:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21744 )

Change subject: IMPALA-13347: Fixes TSAN Thread Leak of Workload Management Thread
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e95967bb6e04470a8900c9ba69080eea8aaa25e
Gerrit-Change-Number: 21744
Gerrit-PatchSet: 9
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2024 15:26:46 +0000
Gerrit-HasComments: No