Bug 160724 - EWS logs file are rotated too quickly
Summary: EWS logs file are rotated too quickly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-09 17:55 PDT by Aakash Jain
Modified: 2016-08-16 15:26 PDT (History)
5 users (show)

See Also:


Attachments
Proposed patch (1.21 KB, patch)
2016-08-09 18:03 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Updated patch (1.68 KB, patch)
2016-08-12 17:19 PDT, Aakash Jain
dbates: review+
Details | Formatted Diff | Diff
Updated patch (1.71 KB, patch)
2016-08-16 14:56 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2016-08-09 17:55:58 PDT
After enabling the recent logging to file in EWS, there are a lot of log files which are very small (< 1kb). This is because we rotate the log file every few hours. This created unnecessary large number of files and makes debugging harder. While rotating the log files, we should also check for file size and rotate the log file only if the size is greater than a threshold.


e.g.: lot of such files of 162B.
-rw-r--r--  1 buildbot  admin   162B Aug  8 12:38 mac-wk2-ews_2016-08-08_12-08.log
-rw-r--r--  1 buildbot  admin   162B Aug  8 13:46 mac-wk2-ews_2016-08-08_14-08.log
-rw-r--r--  1 buildbot  admin   162B Aug  8 15:11 mac-wk2-ews_2016-08-08_15-08.log
Comment 1 Aakash Jain 2016-08-09 18:03:29 PDT
Created attachment 285702 [details]
Proposed patch

For now, I have kept the threshold for file size to be 100kb. We can change it appropriately.
Comment 2 Daniel Bates 2016-08-11 13:47:42 PDT
Comment on attachment 285702 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285702&action=review

> Tools/ChangeLog:7
> +

For your consideration, I suggest elaborating on the new rotation policy.

> Tools/EWSTools/start-queue-mac.sh:58
> +        filesize=$(wc -c <"$QUEUE_NAME.log")

It seems excessive to use wc(1) to count the number of bytes in the file. I suggest that we use stat(1) to retrieve the size of the file from its filesystem metadata (i.e. its inode)
Comment 3 Aakash Jain 2016-08-12 17:19:24 PDT
Created attachment 285983 [details]
Updated patch

Incorporated review comments.
Comment 4 Daniel Bates 2016-08-16 14:35:28 PDT
Comment on attachment 285983 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285983&action=review

> Tools/ChangeLog:9
> +        doesn't have any pending patch, these iterations result in very small amount of 

any => a

OR

"any pending patch" => "any pending patches"

> Tools/ChangeLog:10
> +        logs (1 kb log file). Even if the queue process some patches, logs are few kbs.

kb => KB
kbs => KBs

> Tools/ChangeLog:12
> +        before rotating the log file so that we do not have unnecessarily high number of files.

We should add a remark that explains when we will rotate the logs. That is, when its file size is greater than or equal to 100 KB.

> Tools/EWSTools/start-queue-mac.sh:58
> +        filesize=$(stat -f%z "$QUEUE_NAME.log")

I suggest adding an inline comment "# bytes" to convey the unit of measure. This will save a person time from reading stat(1).

> Tools/EWSTools/start-queue-mac.sh:59
> +        if [ $filesize -ge 102400 ]; then

Note that file size is measured in KB where 1 KB = 1000 bytes. 102400 / 1000 = 102.4 KB. If we want to rotate the logs ever 100 KB then we should check that $filesize >= 1000 * 100 = 100000.
Comment 5 Daniel Bates 2016-08-16 14:37:25 PDT
(In reply to comment #4)
> Note that file size is measured in KB where 1 KB = 1000 bytes. 

I meant to say, file size is measured using units that are powers of 10. So, KB not KiB where 1 KB = 1000 bytes.
Comment 6 Aakash Jain 2016-08-16 14:56:06 PDT
Created attachment 286208 [details]
Updated patch

Incorporated review comments.
Comment 7 WebKit Commit Bot 2016-08-16 15:25:58 PDT
Comment on attachment 286208 [details]
Updated patch

Clearing flags on attachment: 286208

Committed r204527: <http://trac.webkit.org/changeset/204527>
Comment 8 WebKit Commit Bot 2016-08-16 15:26:04 PDT
All reviewed patches have been landed.  Closing bug.