You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2023/12/05 16:41:53 UTC
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20754
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, This patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
custom_cluster_test_suite.py is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False
except for few tests in test_breakpad.py.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid
in TestLogging.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 87 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/1
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#2).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, This patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
custom_cluster_test_suite.py is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False
except for few tests in test_breakpad.py.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid
in TestLogging.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 88 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/2
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14587/ : 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/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 17:17:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/5/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:
http://gerrit.cloudera.org:8080/#/c/20754/5/bin/start-impala-cluster.py@292
PS5, Line 292: we want
> nit: use += [...] to keep consistent with line 295
Done
http://gerrit.cloudera.org:8080/#/c/20754/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/20754/5/tests/common/custom_cluster_test_suite.py@310
PS5, Line 310: Most of test in custom_cluster need to match PID, exc
> nit: most of test in custom_cluster need to match PID, except some test for
Done
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 19:00:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@17
PS2, Line 17: t
> nit: low case
Done
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@24
PS2, Line 24:
: As a minimum exercesise for this new log rotation behavior,
> Why set it to false?
This patch will change the default log rotation behavior to ignore PID.
We can't ignore PID in e2e tests for reason mentioned in commit message.
So I tried to have custom cluster test suite to run with this new log rotation behavior (ignore PID), and I found it can run well except for few tests in test_breakpad.py.
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 23:20:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Wenzhe Zhou, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#7).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercise for this new log rotation behavior,
test_breakpad.py::TestLogging is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid.
- Split TestLogging into two. One run test_excessive_cerr_ignore_pid in
core exploration, while the other run the rest of logging tests in
exhaustive exploration.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 112 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/7
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 7
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#5).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercise for this new log rotation behavior,
test_breakpad.py::TestLogging is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid.
- Split TestLogging into two. One run test_excessive_cerr_ignore_pid in
core exploration, while the other run the rest of logging tests in
exhaustive exploration.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 113 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/5
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
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/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercise for this new log rotation behavior,
test_breakpad.py::TestLogging is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid.
- Split TestLogging into two. One run test_excessive_cerr_ignore_pid in
core exploration, while the other run the rest of logging tests in
exhaustive exploration.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Reviewed-on: http://gerrit.cloudera.org:8080/20754
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 112 insertions(+), 23 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 9
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/6/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:
http://gerrit.cloudera.org:8080/#/c/20754/6/bin/start-impala-cluster.py@293
PS6, Line 293: result += ["-log_rotation_match_pid=1"]
Is '=1' equivalent to '=true'? Seems like an odd way to declare it.
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py@311
PS6, Line 311: cmd.append('--ignore_pid_on_log_rotation')
This seems like excessive plumbing when you could just pass "impalad_args=-log_rotation_match_pid".
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 22:12:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@17
PS2, Line 17: T
nit: low case
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@24
PS2, Line 24: custom_cluster_test_suite.py is modified to invoke
: start-impala-cluster.py with 'log_rotation_match_pid' set to False
Why set it to false?
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 21:54:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@24
PS2, Line 24:
: As a minimum exercise for this new log rotation behavior,
> If run multiple custom cluster tests in a batch, Impala cluster will be res
You're right. I mistakenly thought that custom cluster test put their logs in tmp dir before moving them to logs/custom_cluster_tests/ at the end of tests. Patch set 5 revise the tests.
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 01:13:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14629/ : 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/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 7
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 22:56:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9990/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 23:05:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20754/2//COMMIT_MSG@24
PS2, Line 24:
: As a minimum exercise for this new log rotation behavior,
> This patch will change the default log rotation behavior to ignore PID.
If run multiple custom cluster tests in a batch, Impala cluster will be restarted for each test case. Do you lose some log files in this case if we need log files for each test case?
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 23:31:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 20:00:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#4).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercise for this new log rotation behavior,
custom_cluster_test_suite.py is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False
except for few tests in test_breakpad.py.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid
in TestLogging.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 88 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/4
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/5/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:
http://gerrit.cloudera.org:8080/#/c/20754/5/bin/start-impala-cluster.py@292
PS5, Line 292: .append
nit: use += [...] to keep consistent with line 295
http://gerrit.cloudera.org:8080/#/c/20754/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/20754/5/tests/common/custom_cluster_test_suite.py@310
PS5, Line 310: Some tests under test_breakpad.py still need to match
nit: most of test in custom_cluster need to match PID, except some test for logging
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 18:24:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Sat, 09 Dec 2023 03:34:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#3).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercesise for this new log rotation behavior,
custom_cluster_test_suite.py is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False
except for few tests in test_breakpad.py.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid
in TestLogging.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 88 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/3
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14594/ : 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/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 01:40:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20754/6/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:
http://gerrit.cloudera.org:8080/#/c/20754/6/bin/start-impala-cluster.py@293
PS6, Line 293: result += ["-log_rotation_match_pid=true"]
> Is '=1' equivalent to '=true'? Seems like an odd way to declare it.
Change to '=true'
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py@311
PS6, Line 311: cmd.append('--ignore_pid_on_log_rotation')
> This seems like excessive plumbing when you could just pass "impalad_args=-
I choose to control this by adding option in start-impala-cluster.py because default behavior of impalad after this patch is to ignore PID, but test minicluster still need to match PID. This is why ignore_pid_on_log_rotation default to False in this script and start-impala-cluster.py.
Without dedicated ignore_pid_on_log_rotation option in start-impala-cluster.py, '-log_rotation_match_pid=true' need to be a mandatory option in build_logging_args(), and TestLogging will have its impalad started with confusing successive flags:
"-ignore_pid_on_log_rotation=true -ignore_pid_on_log_rotation=false"
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 7
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 22:35:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 7: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/20754/6/tests/common/custom_cluster_test_suite.py@311
PS6, Line 311: cmd.append('--ignore_pid_on_log_rotation')
> I choose to control this by adding option in start-impala-cluster.py becaus
Oh right, I missed the asymmetry.
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 7
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 22:43:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 8
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2023 23:05:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14604/ : 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/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Dec 2023 19:29:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14585/ : 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/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 17:09:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Joe McDonnell, Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20754
to look at the new patch set (#6).
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
IMPALA-12595: Allow automatic removal of old logs from previous PID
IMPALA-11184 add code to target specific PID for log rotation. This
align with glog behavior and grant safety. That is, it is strictly limit
log rotation to only consider log files made by the currently running
Impalad and exclude logs made by previous PID or other living-colocated
Impalads. The downside of this limit is that logs can start accumulate
in a node when impalad is frequently restarted and is only resolvable by
admin doing manual log removal.
To help avoid this manual removal, this patch adds a backend flag
'log_rotation_match_pid' that relax the limit by dropping the PID in
glob pattern. Default value for this new flag is False. However, for
testing purpose, start-impala-cluster.py will override it to True since
test minicluster logs to a common log directory. Setting
'log_rotation_match_pid' to True will prevent one impalad from
interfering with log rotation of other impalad in minicluster.
As a minimum exercise for this new log rotation behavior,
test_breakpad.py::TestLogging is modified to invoke
start-impala-cluster.py with 'log_rotation_match_pid' set to False.
Testing:
- Add test_excessive_cerr_ignore_pid and test_excessive_cerr_match_pid.
- Split TestLogging into two. One run test_excessive_cerr_ignore_pid in
core exploration, while the other run the rest of logging tests in
exhaustive exploration.
- Pass exhaustive tests.
Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
---
M be/src/common/global-flags.cc
M be/src/common/logging.cc
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/custom_cluster/test_breakpad.py
5 files changed, 112 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/20754/6
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-12595: Allow automatic removal of old logs from previous PID
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20754 )
Change subject: IMPALA-12595: Allow automatic removal of old logs from previous PID
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20754/1/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:
http://gerrit.cloudera.org:8080/#/c/20754/1/bin/start-impala-cluster.py@110
PS1, Line 110: "
flake8: E501 line too long (92 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/20754
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I599799e73f27f941a1d7f3dec0f40b4f05ea5ceb
Gerrit-Change-Number: 20754
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2023 16:42:57 +0000
Gerrit-HasComments: Yes