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.

Sign up for GitHub

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

Jump to bottom

don't block when there are no buffers left #1641

Merged
merged 3 commits into from Jun 30, 2020
Merged

don't block when there are no buffers left #1641

merged 3 commits into from Jun 30, 2020

Conversation

richardstartin
Copy link
Member

No description provided.

@richardstartin richardstartin requested a review from a team as a code owner June 30, 2020 12:23
@richardstartin richardstartin force-pushed the richardstartin/disruptor-handoff branch from 5b6a772 to e2c0859 Compare June 30, 2020 12:52
@richardstartin richardstartin force-pushed the richardstartin/disruptor-handoff branch from e2c0859 to 1d79f43 Compare June 30, 2020 13:03
bantonsson
bantonsson reviewed Jun 30, 2020
View reviewed changes
dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java
@@ -114,6 +116,11 @@ public void onScheduleFlush(final DDAgentWriter agentWriter, final boolean previ
// not recorded
}

@Override
public void onBackedUpTraceBuffer() {
statsd.incrementCounter("trace.buffer.backlog");
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

tylerbenson
tylerbenson approved these changes Jun 30, 2020
View reviewed changes
Copy link
Contributor

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

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

Thanks for catching this. This is likely my fault for not reading/understanding the disruptor API better. I assumed WaitStrategy applied to both producer and consumer, but sadly it seems to be just consumer.

@dougqh
Copy link
Contributor

dougqh commented Jun 30, 2020

Hmm, yes, we should have thought about the no buffer available case a little more carefully.

This seems like a good step in the right direction.
Although, I keep wondering whether we should switch to something other than Disruptor. I feel like what we need to do doesn't really fit into Disruptor's model.

@richardstartin richardstartin force-pushed the richardstartin/disruptor-handoff branch from fa9513e to 25551ca Compare June 30, 2020 16:45
@richardstartin richardstartin force-pushed the richardstartin/disruptor-handoff branch from f39e025 to e2f46a9 Compare June 30, 2020 17:23
@richardstartin richardstartin merged commit 9ed929b into master Jun 30, 2020
@richardstartin richardstartin deleted the richardstartin/disruptor-handoff branch June 30, 2020 18:25
@github-actions github-actions bot added this to the 0.57.0 milestone Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bantonsson bantonsson bantonsson approved these changes

@tylerbenson tylerbenson tylerbenson approved these changes

Assignees
No one assigned
Labels
type: bug
Projects
None yet
Milestone
0.57.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@richardstartin @dougqh @tylerbenson @bantonsson