Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Jan 16, 2026

Summary

Fix potential data race between WriteTraceEvent in ProcessBatch and ReadTraceEvent in the sampling goroutine. Closes #17772.

Performance

Baseline

goos: darwin
goarch: arm64
pkg: github.com/elastic/apm-server/x-pack/apm-server/sampling
cpu: Apple M4 Pro
BenchmarkProcess-14              3096828               357.5 ns/op
BenchmarkProcess-14              3284749               359.1 ns/op
BenchmarkProcess-14              3191538               353.3 ns/op
BenchmarkProcess-14              3333675               345.3 ns/op
BenchmarkProcess-14              3331615               345.4 ns/op
BenchmarkProcess-100             3439828               344.9 ns/op
BenchmarkProcess-100             3526257               325.7 ns/op
BenchmarkProcess-100             3461000               322.2 ns/op
BenchmarkProcess-100             3480249               394.0 ns/op
BenchmarkProcess-100             2877158               608.7 ns/op

Single Mutex

goos: darwin
goarch: arm64
pkg: github.com/elastic/apm-server/x-pack/apm-server/sampling
cpu: Apple M4 Pro
BenchmarkProcess-14              3323389               337.5 ns/op
BenchmarkProcess-14              3482792               347.8 ns/op
BenchmarkProcess-14              3349486               333.5 ns/op
BenchmarkProcess-14              3437163               334.9 ns/op
BenchmarkProcess-14              3362293               333.8 ns/op
BenchmarkProcess-100             3516130               331.9 ns/op
BenchmarkProcess-100             3251674               339.8 ns/op
BenchmarkProcess-100             3432068               333.6 ns/op
BenchmarkProcess-100             3561802               368.2 ns/op
BenchmarkProcess-100             3290845               416.4 ns/op

ShardLockReadWriter

goos: darwin
goarch: arm64
pkg: github.com/elastic/apm-server/x-pack/apm-server/sampling
cpu: Apple M4 Pro
BenchmarkProcess-14              3272188               346.3 ns/op
BenchmarkProcess-14              3415772               330.7 ns/op
BenchmarkProcess-14              3487447               333.2 ns/op
BenchmarkProcess-14              3470158               337.4 ns/op
BenchmarkProcess-14              3467367               338.6 ns/op
BenchmarkProcess-100             3626730               319.7 ns/op
BenchmarkProcess-100             3722044               369.1 ns/op
BenchmarkProcess-100             3123934               349.0 ns/op
BenchmarkProcess-100             3771914               381.9 ns/op
BenchmarkProcess-100             3462182               340.4 ns/op

ShardLockReadWriter with RWMutex

goos: darwin
goarch: arm64
pkg: github.com/elastic/apm-server/x-pack/apm-server/sampling
cpu: Apple M4 Pro
BenchmarkProcess-14              3384834               343.1 ns/op
BenchmarkProcess-14              3284800               338.6 ns/op
BenchmarkProcess-14              3483038               348.2 ns/op
BenchmarkProcess-14              3217903               343.9 ns/op
BenchmarkProcess-14              3444128               334.4 ns/op
BenchmarkProcess-100             3115171               328.8 ns/op
BenchmarkProcess-100             3383545               329.2 ns/op
BenchmarkProcess-100             3069316               328.6 ns/op
BenchmarkProcess-100             3483396               334.0 ns/op
BenchmarkProcess-100             3359440               345.3 ns/op
```<hr>This is an automatic backport of pull request #19948 done by [Mergify](https://mergify.com).

* Add test confirming the potential data race

* Remove unnecessary sleeps

* Add assertion for transaction ids at the end

* Add parent id to transaction2

* Update potential race condition test

* Try fixing race condition

* Fix bug where multiple ongoing trasactions can race to delete first

* Add ShardLockReadWriter

* Panic if numShards <= 0

* Remove unnecessary code

* Use RWMutex

* Make fmt

* Add shard lock on processor level instead

* Make fmt update

* Revert "Make fmt update"

This reverts commit b788c3f.

* Update based on review

(cherry picked from commit 67a5a2b)

# Conflicts:
#	x-pack/apm-server/sampling/processor.go
#	x-pack/apm-server/sampling/processor_test.go
@mergify mergify bot requested a review from a team as a code owner January 16, 2026 09:49
@mergify mergify bot added backport conflicts There is a conflict in the backported pull request labels Jan 16, 2026
@mergify
Copy link
Contributor Author

mergify bot commented Jan 16, 2026

Cherry-pick of 67a5a2b has failed:

On branch mergify/bp/9.1/pr-19948
Your branch is up to date with 'origin/9.1'.

You are currently cherry-picking commit 67a5a2bc.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   internal/beater/monitoringtest/opentelemetry.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   x-pack/apm-server/sampling/processor.go
	both modified:   x-pack/apm-server/sampling/processor_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot added the backport label Jan 16, 2026
@mergify mergify bot added the conflicts There is a conflict in the backported pull request label Jan 16, 2026
@github-actions
Copy link
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify mergify bot mentioned this pull request Jan 16, 2026
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

cc @ericywl

Copy link
Member

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

let's observe the release schedule

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport conflicts There is a conflict in the backported pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants