Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduction of close_older #718

Merged
merged 1 commit into from
Jan 19, 2016
Merged

Introduction of close_older #718

merged 1 commit into from
Jan 19, 2016

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jan 14, 2016

@@ -15,7 +15,8 @@ import (
// Defaults for config variables which are not set
const (
DefaultRegistryFile = ".filebeat"
DefaultIgnoreOlderDuration time.Duration = 24 * time.Hour
DefaultIgnoreOlderDuration time.Duration = 72 * time.Hour
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@urso I suggest to change the default to inifinite (which would be 0). WDYT? Including also @guyboertje (see https://github.com/logstash-plugins/logstash-input-file/pull/87/files#r49697378)

@ruflin ruflin force-pushed the close_older branch 2 times, most recently from 68886de to f186aa8 Compare January 18, 2016 13:43
@ruflin ruflin added the review label Jan 18, 2016
// This ensures we don't skip genuine creations with dead times less than 10s
if h.Stat.Fileinfo.ModTime().Before(p.lastscan) &&
// If ignore_older is disable (by beeing 0) this never gets true
if h.Stat.Fileinfo.ModTime().Before(p.lastscan) && p.config.IgnoreOlderDuration != 0 &&
time.Since(h.Stat.Fileinfo.ModTime()) > p.config.IgnoreOlderDuration {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a long unreadable expression

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I move it into a separate function. Still more cleanup needed (later ...)

@urso
Copy link

urso commented Jan 18, 2016

LGTM

@@ -21,6 +21,7 @@ https://github.com/elastic/beats/compare/1.0.0...master[Check the HEAD diff]
- Rename proc.cpu.user_p with proc.cpu.total_p as includes CPU time spent in kernel space {pull}631[631]

*Filebeat*
- Default config for ignore_older is now inifit instead of 24h, means ignore_older is disabled by default. Use close_older to only close file handlers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo, s/infinit/infinite

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@tsg
Copy link
Contributor

tsg commented Jan 18, 2016

LGTM apart from the small comments above.

…ssues/181

* Set close_older to 1h by defautl
* Set ignore_older to infinity by default (means it is disabled by default)
@ruflin
Copy link
Contributor Author

ruflin commented Jan 19, 2016

@tsg @urso have a look again.

tsg added a commit that referenced this pull request Jan 19, 2016
Introduction of close_older
@tsg tsg merged commit d096631 into elastic:master Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants