Bug 160724

Summary: EWS logs file are rotated too quickly
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, commit-queue, dbates, lforschler
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=160776
Attachments:
Description Flags
Proposed patch
none
Updated patch
dbates: review+
Updated patch none

Aakash Jain
Reported 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
Attachments
Proposed patch (1.21 KB, patch)
2016-08-09 18:03 PDT, Aakash Jain
no flags
Updated patch (1.68 KB, patch)
2016-08-12 17:19 PDT, Aakash Jain
dbates: review+
Updated patch (1.71 KB, patch)
2016-08-16 14:56 PDT, Aakash Jain
no flags
Aakash Jain
Comment 1 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.
Daniel Bates
Comment 2 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)
Aakash Jain
Comment 3 2016-08-12 17:19:24 PDT
Created attachment 285983 [details] Updated patch Incorporated review comments.
Daniel Bates
Comment 4 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.
Daniel Bates
Comment 5 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.
Aakash Jain
Comment 6 2016-08-16 14:56:06 PDT
Created attachment 286208 [details] Updated patch Incorporated review comments.
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2016-08-16 15:26:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.