Skip to content

make locking more fine-grained on client close#14027

Merged
fearful-symmetry merged 4 commits intoelastic:feature/dockerbeatfrom
fearful-symmetry:feature/dockerbeat
Oct 15, 2019
Merged

make locking more fine-grained on client close#14027
fearful-symmetry merged 4 commits intoelastic:feature/dockerbeatfrom
fearful-symmetry:feature/dockerbeat

Conversation

@fearful-symmetry
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry commented Oct 11, 2019

This came out of a discussion with @urso -- Client and pipeline close operations on libbeat can block depending on WaitClose and other settings. We could potentially block other client start operations while the pipelines drained. This attempts to make the locking more fine-grained so we don't hold a mutex while queues drain.

@fearful-symmetry fearful-symmetry added the Team:Integrations Label for the Integrations team label Oct 11, 2019
@fearful-symmetry fearful-symmetry requested review from a team and urso October 11, 2019 15:45
@fearful-symmetry fearful-symmetry self-assigned this Oct 11, 2019
Copy link
Copy Markdown
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

I left a comment about potential problem. Blocking this until you can check it 👍

@fearful-symmetry
Copy link
Copy Markdown
Contributor Author

@sayden fixed!

Copy link
Copy Markdown
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Nice! Awesome :)

@fearful-symmetry fearful-symmetry merged commit 6c25ff1 into elastic:feature/dockerbeat Oct 15, 2019
if err != nil {
return nil, errors.Wrap(err, "error loading pipeline")
}
pm.registerPipeline(pipeline, hashstring)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This pattern looks like a race, potentially creating zombie-pipelines. At least the if block should be protected and there should be another check (similar to double-checked locking pattern).

Does docker synchronize API calls on logging plugins? In this case we might be safe here.

@urso
Copy link
Copy Markdown

urso commented Oct 15, 2019

What happened to the notice file? Given the change itself I wonder why we need to update it in this PR.

fearful-symmetry added a commit to fearful-symmetry/beats that referenced this pull request Nov 1, 2019
* make locking more fine-grained on client close
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* make locking more fine-grained on client close
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants