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

Cherry-pick #8653 to 6.x: Fix a race condition on add_host_metadata #8676

Merged
merged 1 commit into from
Oct 23, 2018

Conversation

jsoriano
Copy link
Member

Cherry-pick of PR #8653 to 6.x branch. Original message:

Not sure if adding a mutex at this level will be too blocking for events processing, I have also though on other options to reduce contention on reads:

  • Using a RWLock, but we'd still need to write-lock first to check expiration and update
  • Using a RWLock and a go-routine to do periodic updates, but afaik we wouldn't have a way to stop it if the processor is removed.

Continues with #8223

add_host_metadata keeps a cache of the host data collected, this cache
is now updated atomically.

(cherry picked from commit 74b9c6c)
@jsoriano
Copy link
Member Author

jenkins, test this again please

@@ -40,7 +40,7 @@ https://github.com/elastic/beats/compare/v6.4.0...6.x[Check the HEAD diff]
- Deregister pipeline loader callback when inputsRunner is stopped. {pull}[7893][7893]
- Add backoff support to x-pack monitoring outputs. {issue}7966[7966]
- Removed execute permissions systemd unit file. {pull}7873[7873]
- Fix a race condition with the `add_host_metadata` and the event serialization. {pull}8223[8223]
- Fix a race condition with the `add_host_metadata` and the event serialization. {pull}8223[8223] {pull}8653[8653]
Copy link
Contributor

Choose a reason for hiding this comment

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

Double pull? What does it mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

This change is following another related fix (#8223), so both PRs are about the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification 🙂

Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

CI failure does not look related.

@jsoriano jsoriano merged commit 38cbe9c into elastic:6.x Oct 23, 2018
@jsoriano jsoriano deleted the backport_8653_6.x branch October 23, 2018 10:42
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.

3 participants