-
Notifications
You must be signed in to change notification settings - Fork 2.1k
LogCleaner: avoid scanning logs too frequently #776
Conversation
Codecov Report
@@ Coverage Diff @@
## master #776 +/- ##
==========================================
+ Coverage 72.65% 72.72% +0.07%
==========================================
Files 17 17
Lines 3247 3256 +9
==========================================
+ Hits 2359 2368 +9
Misses 888 888
Continue to review full report at Codecov.
|
sergiud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Please take a look at the contribution guide and make sure to follow it. Particularly, you must sign the CLA.
|
|
||
| bool enabled_; | ||
| unsigned int overdue_days_; | ||
| int64 next_cleanup_time_; // cycle count at which to clean overdue log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As before: why using a signed integer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As before: I use integer because another timer ( e.g. next_flush_time_ ) use integer, too.
33e59bd to
e7ccdf9
Compare
|
@fdgkhdkgh Some comments are still open. Please let me know if you need some clarification. |
e7ccdf9 to
4abfb55
Compare
|
@fdgkhdkgh Thank you for adding this! |
|
This looks good to me now (unless @aesophor has any objections). @fdgkhdkgh Would you like to add yourself to the |
|
LGTM. Thanks so much! 😄 |
|
Thanks! |
This patch is for the performance issue : #753.
Originally, "LogCleaner" scanned all the log directories every time we flush. In the common case, we flush every 30 seconds and then called "LogCleaner::Run". But if we flush every time we do "LogFileObject::Write" like the test code from @kimi20071025, we'll call "LogCleaner::Run" so many times.
In this patch, I maintain an additional timer
next_cleanup_time_( thank @aesophor for the advice ), and avoid scanning the logs too frequently.