Change in vdsm[master]: vdsm-logrotate.conf: Add copytruncate

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has uploaded a new change for review.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

vdsm-logrotate.conf: Add copytruncate

Rotating the log files is not enough, VDSM keep it open and
appending entries.

Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Bug-URL: https://bugzilla.redhat.com/show_bug.cgi?id=928217
Signed-off-by: Douglas Schilling Landgraf 
---
M vdsm/vdsm-logrotate.conf.in
1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/13499/1

diff --git a/vdsm/vdsm-logrotate.conf.in b/vdsm/vdsm-logrotate.conf.in
index ce71015..dc05f16 100644
--- a/vdsm/vdsm-logrotate.conf.in
+++ b/vdsm/vdsm-logrotate.conf.in
@@ -1,5 +1,6 @@
 /var/log/vdsm/*.log {
     rotate 100
+    copytruncate
     missingok
     size 15M
     compress

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1817/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1766/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1766/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1817/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: Verified

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server

Zhou Zheng Sheng has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/vdsm-logrotate.conf.in
Line 1: /var/log/vdsm/*.log {
Line 2:     rotate 100
Line 3:     copytruncate
I think copytruncate is not necessary.  In vdsm/logger.conf.in, the handler for vdsm.log is logging.handlers.WatchedFileHandler [1]. This handler examines the inode number and device number of vdsm.log before every write operation.  There is a default option "create" in /etc/logrotate.conf. After logrotate does its job, it creates a new and empty vdsm.log. The inode number of the new vdsm.log is changed, so WatchedFileHandler will close and re-open the vdsm.log .

[1] http://docs.python.org/2/library/logging.handlers.html#watchedfilehandler
Line 4:     missingok
Line 5:     size 15M
Line 6:     compress
Line 7:     compresscmd /usr/bin/xz

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1:

Hello Zhou,

I do agree with and that's my thought as well (I have noticed with ls -i) during my tests. However, I don't think copytruncate hurt us.

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

Zhou Zheng Sheng has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: No score

OK.

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

Zhou Zheng Sheng has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/vdsm-logrotate.conf.in
Line 1: /var/log/vdsm/*.log {
Line 2:     rotate 100
Line 3:     copytruncate
> From the bugzilla link I can see, the size of /var/log/vdsm/vdsm.log.1 is normal, but the size of /var/log/vdsm/libvirt.log.1 is huge.

/var/log/vdsm/libvirt.log is not written by vdsm, it's written by libvirt, so vdsm/logger.conf.in does not apply to this file, no WatchedFileHandler available for this file. This file needs copytruncate. Maybe other log files need this as well, as long as they are not configured by vdsm/logger.conf.in .

However with copytruncate, there is a small time window between copying the file and truncating it, some log message may be lost.
Line 4:     missingok
Line 5:     size 15M
Line 6:     compress
Line 7:     compresscmd /usr/bin/xz

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

Mark Wu has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm/vdsm-logrotate.conf.in
Line 1: /var/log/vdsm/*.log {
Line 2:     rotate 100
Line 3:     copytruncate
Zhengsheng,
Thanks for taking time to look into this problem. As you said, the file logging handler in libvirt can't detect the file change. So it will still keep the original log open, which make the disk space can't be freed when logrotate removes it.  I didn't notice that the lograte's configuration will impact on other logs except vdsm.  That's my fault.  I apologize for that. 
But I can't explain why vdsm.log.1 was still kept open. It's expected to be closed by WatchedFileHandler.

Anyway, copytruncate is necessary for libvirt and no harm with WatchedFileHandler.
Line 4:     missingok
Line 5:     size 15M
Line 6:     compress
Line 7:     compresscmd /usr/bin/xz

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: I would prefer that you didn't submit this

The bug here is part of libvirt log configuration, in VDSM we use logger handler that takes care of switching open files after rotate as you mentioned on the patch.

Changing the configuration of logrotate will fix the issue, but also add side effect that some logs might disappear due to the truncate. If we can fix the real cause of that open file in libvirt logger, it will be much nicer. Is it possible? did you check with libvirt guys?

Also I wonder, why if we decide to add copytruncate we still need the WatchedFileHandler, maybe we can use FileHandler and avoid other bug that related to the emit check (https://bugzilla.redhat.com/show_bug.cgi?id=919163)?

Keeping the watchedFileHandler and copytrucate without handling the cause in libvirt seems like too much work for logrotate and logger module..

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has abandoned this change.

Change subject: vdsm-logrotate.conf: Add copytruncate
......................................................................

Patch Set 1: Abandoned

http://gerrit.ovirt.org/#/c/13728/

--
To view, visit http://gerrit.ovirt.org/13499
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie018b2039eac565cfdb5874c664b177d6fad64fa
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server