Skip to content

Replace ArrayBlockingQueue with park/unpark for BatchSpanProcessor$Worker - #8240

Open
Khepu wants to merge 4 commits into
open-telemetry:mainfrom
Khepu:main
Open

Replace ArrayBlockingQueue with park/unpark for BatchSpanProcessor$Worker#8240
Khepu wants to merge 4 commits into
open-telemetry:mainfrom
Khepu:main

Conversation

@Khepu

@Khepu Khepu commented Apr 1, 2026

Copy link
Copy Markdown

Browsing through the codebase I saw the ArrayBlockingQueue and thought I could do something better. Running benchmarks locally did produce better results though they weren't as consistent as I would have liked.

The same changes could also be applied to BatchLogRecordProcessor.

Khepu added 2 commits April 1, 2026 21:40
Bypassing all the complexity that comes with ABQ should come with improved performance
@Khepu
Khepu requested a review from a team as a code owner April 1, 2026 19:47
@linux-foundation-easycla

linux-foundation-easycla Bot commented Apr 1, 2026

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov

codecov Bot commented Apr 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (c856c2b) to head (8970e4d).
⚠️ Report is 294 commits behind head on main.

Files with missing lines Patch % Lines
...telemetry/sdk/trace/export/BatchSpanProcessor.java 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8240      +/-   ##
============================================
- Coverage     90.32%   90.32%   -0.01%     
+ Complexity     7654     7653       -1     
============================================
  Files           843      843              
  Lines         23080    23076       -4     
  Branches       2312     2312              
============================================
- Hits          20848    20843       -5     
+ Misses         1516     1514       -2     
- Partials        716      719       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Making the worker extend thread was the only sensible way of having it keep a constant reference to its thread. Everything else I tried was quite hacky
@zeitlinger

Copy link
Copy Markdown
Member

Thanks for the contribution!

Please run the bench before/after and add results here - see #8271 on how to do it.

@Khepu

Khepu commented Apr 21, 2026

Copy link
Copy Markdown
Author

I've been running the benchmarks and I wonder if I am doing something wrong because every one of the BatchSpanProcessor benchmark files that does keep track of metrics for spans exported/dropped they end up being 0. Tested this before my changes too just to make sure I didn't break this by accident.

@jack-berg

Copy link
Copy Markdown
Member

Which ones did you try? Benchmarks collect dust and eventually get out of date, especially the ones not regularly used. Would RecordSpanBenchmark show the changes? That's one of the few that we run on an ongoing basis and publish results to https://open-telemetry.github.io/opentelemetry-java/benchmarks/

@Khepu

Khepu commented Apr 23, 2026

Copy link
Copy Markdown
Author

That one I can run just fine. I have been trying to run these 2:

SpanRecordBenchmark won't help here. I was trying the above 2 because all I am trying to improve is throughput of exported spans under load. The only metrics that could prove some improvement/regression here would be the number of exported and dropped spans.

I will try spending some time to figure out why those metrics end up being zero on those benchmarks.

This was referenced May 4, 2026
This was referenced May 26, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 18, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-08-04 17:08 UTC

Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):

  • Top-level threads: 1
Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 29, 2026

Copy link
Copy Markdown

Hi @Khepu — just a friendly reminder that this pull request is waiting on you.

There are still items that need your attention. See the dashboard status comment for the full list. You don't need to push a code change to hand it back — replying to move each discussion forward is enough, whether that's answering a question, explaining why no change is needed, or asking a follow-up. The dashboard then automatically routes it back to reviewers.

If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants