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 #19821 to 7.x: libbeat/publisher/pipeline: fix data races #19865

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

axw
Copy link
Member

@axw axw commented Jul 14, 2020

Cherry-pick of PR #19821 to 7.x branch. Original message:

What does this PR do?

Fix how we pass the initial queue consumer into eventConsumer.loop;
we were referencing c.consumer in a background goroutine, which can
race with updates to the consumer.

Update tests to properly load atomic variables. Changed serially updated
numEvents vars to basic, non-atomic types.

Why is it important?

I'm not sure if this particular data race would cause any issues in production,
but we (apm-server) rely on the race detector to pick up real issues. This is
causing our tests to fail.

Checklist

  • My code follows the style guidelines of this project
    - [ ] I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation
    - [ ] I have made corresponding change to the default configuration files
    - [ ] I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

(Does this change need a changelog entry? This is a fix to a recently merged change.)

How to test this PR locally

go test -race ./libbeat/publisher/pipeline

Fix how we pass the initial queue consumer into
eventConsumer.loop; we were referencing c.consumer
in a background goroutine, which can race with
updates to the consumer.

Update tests to properly load atomic variables.
Changed serially updated numEvents vars to basic,
non-atomic types.

(cherry picked from commit ebacd3b)
@axw axw added [zube]: In Review backport Team:Services (Deprecated) Label for the former Integrations-Services team labels Jul 14, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jul 14, 2020
@andrewkroh
Copy link
Member

run tests

@axw
Copy link
Member Author

axw commented Jul 15, 2020

Failures are unrelated - merging.

@axw axw merged commit 5c28639 into elastic:7.x Jul 15, 2020
@axw axw deleted the backport_19821_7.x branch July 15, 2020 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Team:Services (Deprecated) Label for the former Integrations-Services team [zube]: Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants