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 2021/11/08 20:43:42 UTC

[Impala-ASF-CR] IMPALA-5256: Force log rotation when max log size exceeded

Hello Joe McDonnell, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-5256: Force log rotation when max_log_size exceeded
......................................................................

IMPALA-5256: Force log rotation when max_log_size exceeded

Impala daemons allow STDOUT/STDERR redirection into INFO/ERROR log
respectively through redirect_stdout_stderr startup flag. If
redirect_stdout_stderr is true, daemons redirect STDOUT/STDERR stream to
write into the log file symlink created by glog. There are two problems
with this approach:

1. Glog updates the symlink to point to the new log file when it does
   log rotation. However, Impala is not aware that the symlink point to
   a different file. So cout/cerr write still goes to the oldest log
   file.

2. When there is a lot of write activity to cout/cerr, the log file can
   grow big. However, glog is not aware of STDOUT/STDERR activity. It
   only counts the message bytes written to glog (LOG(INFO),
   LOG(ERROR)). Thus, it only uses its internal bytes count when
   deciding to rotate the logs.

This commit addresses the issue by monitoring the log file size every
second. If Impala sees that the log file has exceeded max_log_size, it
will call google::FlushLogFiles(), ahead of logbufsecs. If the log file
stays big after the flush, we will force the glog to rotate the log.
Since there is no direct way to force glog to rotate, we do this by
changing the log extension to random extension through
google::SetLogFilenameExtension(), and immediately return them to
extensionless (empty string extension).

We also check periodically whether the log file symlink has pointed to a
new file. If it has changed, we reattach the STDOUT/STDERR stream to the
new log file.

Testing:
- Pass the core test.
- Add new exhaustive test TestLogging::test_excessive_cerr.

Change-Id: I1b94727180354fe69989ebf3cd1a8f8cda1cf0c3
---
M be/src/common/init.cc
M be/src/common/logging.cc
M be/src/common/logging.h
M be/src/util/filesystem-util.cc
M be/src/util/filesystem-util.h
M tests/custom_cluster/test_breakpad.py
6 files changed, 254 insertions(+), 35 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b94727180354fe69989ebf3cd1a8f8cda1cf0c3
Gerrit-Change-Number: 17997
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>