Navigation Menu

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

Fix issue when clean_removed and clean_inactive were used together #2750

Merged
merged 1 commit into from Oct 11, 2016

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Oct 11, 2016

If clean_removed and clean_inactive were used together, states were not directly removed from the registry. The clean_inactive option was updating the TTL after clean_removed was applied. This lead to the issue that during every run clean_removed updated the state and clean_inactive reset the TTL. In the end removed files were never purged from the registry if both options were used together.

  • Test added to verify fix
  • Updated incorrect log message

Closes #2646

…at states were not directly removed from the registry

The clean_inactive option was updating the TTL after clean_removed was applied. This lead to the issue that during every run clean_removed updated the state and clean_inactive reset the TTL. In the end removed files were never purged from the registry if both options were used together.

* Test added to verify fix
* Updated incorrect log message

Closes elastic#2646
@ruflin ruflin added bug review Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. labels Oct 11, 2016
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm not super familiar with the state management in FB so it would be good to have another set of eyes.

// Add ttl if cleanOlder is enabled
if p.config.CleanInactive > 0 {
// Add ttl if cleanOlder is enabled and TTL is not already 0
if p.config.CleanInactive > 0 && event.State.TTL != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if the new check should be for event.State.TTL > 0 since event.State.TTL starts as a -1 * time.Second.

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea here is that TTL is reset every time the state is updated. As all new states are initialies with -1 this would mean these states would never get a TTL according to clean_inactive. My first idea was to have state.Remove = true as a flag for states that can be removed but it would blow up the change. But this would clearly show the difference between TTL and states that should be removed. TTL = 0 is like an alias for the Removed option and is only set when one of the clean options is reached.

@andrewkroh andrewkroh merged commit e1a915a into elastic:master Oct 11, 2016
@ruflin ruflin removed the needs_backport PR is waiting to be backported to other branches. label Oct 11, 2016
@ruflin ruflin deleted the fix-clean_removed branch October 11, 2016 14:27
ruflin added a commit to ruflin/beats that referenced this pull request Oct 11, 2016
…at states were not directly removed from the registry (elastic#2750)

The clean_inactive option was updating the TTL after clean_removed was applied. This lead to the issue that during every run clean_removed updated the state and clean_inactive reset the TTL. In the end removed files were never purged from the registry if both options were used together.

* Test added to verify fix
* Updated incorrect log message

Closes elastic#2646
tsg pushed a commit that referenced this pull request Oct 12, 2016
…at states were not directly removed from the registry (#2750) (#2754)

The clean_inactive option was updating the TTL after clean_removed was applied. This lead to the issue that during every run clean_removed updated the state and clean_inactive reset the TTL. In the end removed files were never purged from the registry if both options were used together.

* Test added to verify fix
* Updated incorrect log message

Closes #2646
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…at states were not directly removed from the registry (elastic#2750) (elastic#2754)

The clean_inactive option was updating the TTL after clean_removed was applied. This lead to the issue that during every run clean_removed updated the state and clean_inactive reset the TTL. In the end removed files were never purged from the registry if both options were used together.

* Test added to verify fix
* Updated incorrect log message

Closes elastic#2646
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filebeat 5.0.0-beta1 not always removing deleted files from registry
2 participants