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

Introduce clean_removed #1922

Merged
merged 1 commit into from
Jul 5, 2016
Merged

Introduce clean_removed #1922

merged 1 commit into from
Jul 5, 2016

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Jun 28, 2016

No description provided.


// States is older then ttl
if int64(time.Since(state.Timestamp).Nanoseconds()/int64(time.Microsecond)) > state.Ttl {
Copy link

Choose a reason for hiding this comment

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

can we transform state.TTL to be time.Duration. Not happy with conversions like to nanos and devide by Microseconds. Which unit is TTL in?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just converted and now realised that it seems like this breaks the json conversion. Any trick here?

    TTL         time.Duration     `json:"ttl"`

Copy link
Member Author

Choose a reason for hiding this comment

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

time.Duration changes were done in #1915

@@ -56,6 +56,7 @@ https://github.com/elastic/beats/compare/v5.0.0-alpha3...master[Check the HEAD d
- Introduce close_removed and close_renamed harvester options {issue}1600[1600]
- Introduce close_eof harvester options {issue}1600[1600]

- Add clean_removed config option
Copy link

Choose a reason for hiding this comment

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

newline on purpose?

#close_eof: false

# Files for the modification data is older then clean_older the state from the registry is removed
# By default this is disabled.
#clean_older: 0

# Removes the state for file which cannot be found on disk anymore immidiately
Copy link

Choose a reason for hiding this comment

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

s/immidiately/immediately/

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@ruflin ruflin force-pushed the fb-clean-removed branch 3 times, most recently from 43a9bfe to b4763ba Compare July 4, 2016 19:46
@ruflin ruflin removed the blocked label Jul 4, 2016
@@ -93,7 +93,7 @@ func (s *States) Cleanup() {

for _, state := range s.states {
ttl := state.TTL
if ttl >= 0 && currentTime.Sub(state.Timestamp) > ttl {
if ttl >= 0 && currentTime.Sub(state.Timestamp) >= ttl {
Copy link

Choose a reason for hiding this comment

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

how about: if ttl==0 || currentTime.Sub(state.Timestamp) > ttl ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would be ok with ttl==0 || currentTime.Sub(state.Timestamp) >= ttl I had one test case where state.Timestamp == currentTimestamp ! Not sure how this was able to happen (windows ...)

@urso urso merged commit 4ae89f2 into elastic:master Jul 5, 2016
@ruflin ruflin deleted the fb-clean-removed branch July 18, 2016 11:14
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.

2 participants