Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

RemoteReporter throws exception when commandQueue is full #178

Closed
mabn opened this issue May 11, 2017 · 1 comment · Fixed by #180
Closed

RemoteReporter throws exception when commandQueue is full #178

mabn opened this issue May 11, 2017 · 1 comment · Fixed by #180

Comments

@mabn
Copy link
Contributor

mabn commented May 11, 2017

RemoteReporter uses ArrayBlockingQueue to store commands, but adding new commands is done with ArrayBlockingQueue.add(x) method which:

Inserts the specified element into this queue if it is possible to do so immediately without violating capacity restrictions, returning true upon success and throwing an IllegalStateException if no space is currently available. When using a capacity-restricted queue, it is generally preferable to use offer.

Right now RemoteReporter.report checks if queue is full but it is not thread-safe.

It would be better to avoid IllegalStateException by using RemoteReporter.offer(x), and if it fails (returns false) increment reporterDropped metric.

New version:

  public void report(Span span) {
    // Its better to drop spans, than to block here
    boolean added = commandQueue.offer(new AppendCommand(span));
    if(!added) {
        metrics.reporterDropped.inc(1);
    }
  }

The other adds should also offer instead.

@yurishkuro
Copy link
Member

Fixed by #180

Thanks, @mabn

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants