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 2024/04/22 23:38:29 UTC

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21345


Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

'test_query_log_table_flush_on_shutdown' is unchanged because it
explicitly triggers graceful shutdown during the test.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
2 files changed, 3 insertions(+), 2 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py@720
PS1, Line 720: client2 = self.create_client_for_nth_impalad(1, vector.get_value('protocol'))
> impalad_graceful_shutdown stays False for this test.
That does seem like it could be an issue. I'll look into ensuring it also shuts down gracefully, just can't use impalad_graceful_shutdown as is.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 20:56:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399
PS10, Line 399: if (local_be_desc->is_quiescing()) {
> Oh actually this is way more straight-forward. The next clause is "else if 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:03:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
3 files changed, 13 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 13: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:23:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

Skips logging DFATAL if the local backend executor has not been added to
an executor group, as happens on dedicated coordinators. Avoids

  Tried to remove non-existing backend from per-host list: 127.0.0.1:...

and a crash with minidump when doing graceful shutdown on dedicated
coordinators.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/executor-group.cc
M be/src/scheduling/executor-group.h
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
6 files changed, 38 insertions(+), 20 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 21:39:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

Skips logging DFATAL if the local backend executor has not been added to
an executor group, as happens on dedicated coordinators. Avoids

  Tried to remove non-existing backend from per-host list: 127.0.0.1:...

and a crash with minidump when doing graceful shutdown on dedicated
coordinators.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/executor-group.cc
M be/src/scheduling/executor-group.h
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
6 files changed, 36 insertions(+), 20 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 10:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:13:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that lowers
'query_log_write_interval_s' or 'query_log_max_queued'.

Lowers grace period on test_query_log_table_flush_on_shutdown because
ShutdownWorkloadManagement() is not started until the grace period ends.

Updates "Adding/Removing local backend" to only apply to executors. It
was only added for executors, but would be removed on dedicated
coordinators as well (resulting in a DFATAL message during graceful
shutdown).

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M be/src/scheduling/cluster-membership-mgr.cc
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
3 files changed, 21 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/12
-- 
To view, visit http://gerrit.cloudera.org:8080/21345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 18:37:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 17:06:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 23:48:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399
PS10, Line 399: VLOG(1) << "Removing local backend from group " << group.DebugString
> Yeah, I'll update this to only pass false if it's a dedicated coordinator.
Oh actually this is way more straight-forward. The next clause is "else if is_executor", so only executors add local backends. I'm going to clean this up.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:58:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 5: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@222
PS4, Line 222: method
> Later lines expect method to not be None, so I suspect this was overly defe
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 17:42:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py@720
PS1, Line 720: client2 = self.create_client_for_nth_impalad(1, vector.get_value('protocol'))
impalad_graceful_shutdown stays False for this test.

Even if first impalad is gracefully shutting down through L720, what will ensure second impalad is also gracefully shutting down and flush query at L724?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Apr 2024 23:58:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@222
PS4, Line 222: method
method is used before checking if it's not None in next line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 01:12:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 13:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:23:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 6:

> Patch Set 6:
> 
> Most of the tests modified here ran into
> 
>   Tried to remove non-existing backend from per-host list
> 
> and crashed during graceful shutdown. I don't see any existing bugs filed for this, so digging into it.

All the failures come from dedicated coordinators going through graceful shutdown. Seems like a latent bug we haven't tested.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 18:42:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@120
PS4, Line 120: None)
> Perhaps this should change to None, and the check in L153 change to "is not
Done


http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@222
PS4, Line 222: method
> method is used before checking if it's not None in next line
Later lines expect method to not be None, so I suspect this was overly defensive coding. Going to remove the 'method is not None' check.


http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@226
PS4, Line 226: assert impalad.get_pid() is None
> Can you put assert message? Maybe like:
Done


http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@227
PS4, Line 227:     "imp
> Can change to else:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 16:48:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 16:54:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21345/2//COMMIT_MSG@10
PS2, Line 10: 'sys.impala_query_log' to avoid leaving the table locked in HMS by a
> Is it still an issue if the coordinator crashes? This could happen in reali
Any Iceberg table operation holding an HMS lock when Impala crashes would encounter the same issue. My understanding is HMS has a timeout to later release the lock.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 22:07:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py@223
PS3, Line 223: allow_stopped=True
> What if we keep allow_stopped=False by default, but override teardown_metho
We haven't previously checked. Without graceful shutdown, impalad is only stopped when restarting with start_impala_cluster, which doesn't care if any are there.

However I think I agree it would be useful to catch, so I'll look into improving it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 23:14:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py@223
PS3, Line 223: allow_stopped=True
What if we keep allow_stopped=False by default, but override teardown_method method in test_query_live.py and test_query_log.py?
I'm concern if we will miss unexpected impalad termination if we ignore missing pid by default for all custom cluster tests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 23:05:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 18:39:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 23:58:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/21345/2//COMMIT_MSG@10
PS2, Line 10: 'sys.impala_query_log' to avoid leaving the table locked in HMS by a
Is it still an issue if the coordinator crashes? This could happen in reality. It's bad if a coordinator restart can't recover it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 22:03:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 6:

Most of the tests modified here ran into

  Tried to remove non-existing backend from per-host list

and crashed during graceful shutdown. I don't see any existing bugs filed for this, so digging into it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 18:34:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399
PS10, Line 399: // Local backend may not have been added in a dedicated coordinator.
> What is "Local backend" here? Different impalad in the same host, or This i
Yeah, I'll update this to only pass false if it's a dedicated coordinator.

"Local backend" means this impalad. I'm not entirely sure why it 'local_be_desc->executor_groups()' becomes non-empty on dedicated coordinators; it may be that during tests it doesn't have long enough to register a membership update after executor groups are added.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:32:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 11:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:16:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that lowers
'query_log_write_interval_s' or 'query_log_max_queued'.

Lowers grace period on test_query_log_table_flush_on_shutdown because
ShutdownWorkloadManagement() is not started until the grace period ends.

Skips logging DFATAL if the local backend executor has not been added to
an executor group, as happens on dedicated coordinators. Avoids

  Tried to remove non-existing backend from per-host list: 127.0.0.1:...

and a crash with minidump when doing graceful shutdown on dedicated
coordinators.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/executor-group.cc
M be/src/scheduling/executor-group.h
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
5 files changed, 27 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/10
-- 
To view, visit http://gerrit.cloudera.org:8080/21345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Apr 2024 00:03:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 19:41:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/21345/10/be/src/scheduling/cluster-membership-mgr.cc@399
PS10, Line 399: // Local backend may not have been added in a dedicated coordinator.
What is "Local backend" here? Different impalad in the same host, or This impalad when it is not a dedicated coordinator?

Can we add DCHECK to confirm that this is indeed an impalad configured as dedicated coordinator, or set expect_exists param in RemoveExecutorAndGroup to false if and only if this is a dedicated coordinator?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 10
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:29:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
4 files changed, 9 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/3/tests/common/custom_cluster_test_suite.py@223
PS3, Line 223: raceful_shutdown:
> We haven't previously checked. Without graceful shutdown, impalad is only s
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 23:34:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@120
PS4, Line 120: False
Perhaps this should change to None, and the check in L153 change to "is not None".


http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@226
PS4, Line 226: assert impalad.get_pid() is None
Can you put assert message? Maybe like:
"i-th impalad is alive, but not listed in impalad_graceful_shutdown list. Expect the test to terminate it before teardown."


http://gerrit.cloudera.org:8080/#/c/21345/4/tests/common/custom_cluster_test_suite.py@227
PS4, Line 227: continue
Can change to else:



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 23:56:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that sets
'query_log_write_interval_s' or 'query_log_max_queued'.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
3 files changed, 18 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Apr 2024 18:39:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/21345/1/tests/custom_cluster/test_query_log.py@720
PS1, Line 720: client2 = self.create_client_for_nth_impalad(1, vector.get_value('protocol'))
> That does seem like it could be an issue. I'll look into ensuring it also s
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 21:15:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 21:02:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 12: Code-Review+2

Looks good. Thanks for digging into it!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:12:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21345 )

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that lowers
'query_log_write_interval_s' or 'query_log_max_queued'.

Lowers grace period on test_query_log_table_flush_on_shutdown because
ShutdownWorkloadManagement() is not started until the grace period ends.

Updates "Adding/Removing local backend" to only apply to executors. It
was only added for executors, but would be removed on dedicated
coordinators as well (resulting in a DFATAL message during graceful
shutdown).

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Reviewed-on: http://gerrit.cloudera.org:8080/21345
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/cluster-membership-mgr.cc
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
3 files changed, 21 insertions(+), 14 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 27 Apr 2024 03:19:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Riza Suminto, Jason Fehr, Zoltan Borok-Nagy, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................

IMPALA-12997: Use graceful shutdown for query log tests

Uses graceful shutdown for all tests that might insert into
'sys.impala_query_log' to avoid leaving the table locked in HMS by a
SIGTERM. That's primarily any test that lowers
'query_log_write_interval_s' or 'query_log_max_queued'.

Lowers grace period on test_query_log_table_flush_on_shutdown because
ShutdownWorkloadManagement() is not started until the grace period ends.

Skips logging DFATAL if the local backend executor has not been added to
an executor group, as happens on dedicated coordinators. Avoids

  Tried to remove non-existing backend from per-host list: 127.0.0.1:...

and a crash with minidump when doing graceful shutdown on dedicated
coordinators.

Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
---
M be/src/scheduling/cluster-membership-mgr.cc
M be/src/scheduling/executor-group.cc
M be/src/scheduling/executor-group.h
M tests/custom_cluster/test_query_live.py
M tests/custom_cluster/test_query_log.py
5 files changed, 29 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21345/11
-- 
To view, visit http://gerrit.cloudera.org:8080/21345
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 11
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12997: Use graceful shutdown for query log tests

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

Change subject: IMPALA-12997: Use graceful shutdown for query log tests
......................................................................


Patch Set 12:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia123c53a952a77ff4a9c02736b5717ccaa3566dc
Gerrit-Change-Number: 21345
Gerrit-PatchSet: 12
Gerrit-Owner: Michael Smith <mi...@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: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Apr 2024 22:27:42 +0000
Gerrit-HasComments: No