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 race on shared maps in global fields #6947

Merged
merged 2 commits into from
May 3, 2018

Conversation

urso
Copy link

@urso urso commented Apr 25, 2018

On publish fields are added to an event in this order:

  • local/global configured fields
  • dynamic fields
  • "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the beat namespace or if dynamic fields are enabled.

The dynamic fields support has been changed to dynamically test if a clone is required or not.

@urso urso force-pushed the fix-fields-overwrite branch from da43b1b to 96c30fa Compare April 25, 2018 13:02
@ph ph self-assigned this Apr 25, 2018
On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.
@urso urso force-pushed the fix-fields-overwrite branch from 96c30fa to 2bf300c Compare May 3, 2018 12:24
@@ -53,7 +53,8 @@ func newProcessorPipeline(
localProcessors = makeClientProcessors(config)
)

needsCopy := global.alwaysCopy || localProcessors != nil || global.processors != nil
// needsCopy := global.alwaysCopy || localProcessors != nil || global.processors != nil
needsCopy := true
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks weird. Is the intention to always copy? Or can the needsCopy value change to false in other parts of the code?

Copy link
Author

Choose a reason for hiding this comment

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

oh, some leftover from testing. :(

@tsg tsg merged commit ffa812b into elastic:master May 3, 2018
@urso urso added the needs_backport PR is waiting to be backported to other branches. label May 3, 2018
@urso urso deleted the fix-fields-overwrite branch May 3, 2018 13:06
@urso urso added v6.2.5 and removed needs_backport PR is waiting to be backported to other branches. labels May 3, 2018
urso pushed a commit to urso/beats that referenced this pull request May 3, 2018
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit ffa812b)
@urso urso added the v6.3.0 label May 3, 2018
urso pushed a commit to urso/beats that referenced this pull request May 3, 2018
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit ffa812b)
ruflin pushed a commit that referenced this pull request May 4, 2018
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit ffa812b)
ruflin pushed a commit that referenced this pull request May 4, 2018
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit ffa812b)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit 7166823)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Fix race on shared maps in global fields

On publish fields are added to an event in this order:
- local/global configured fields
- dynamic fields
- "beat" metadata

When merging the fields, shared structures must not be overwritten or
updated concurrently. This is enforced by cloning the original fields
structure before applying updates.

This adds missing Clone operations if configured fields add new
fields to the `beat` namespace or if dynamic fields are enabled.

* Remove hard coded bool from testing

(cherry picked from commit 7166823)
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