Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

Improvements to trace writer and payload queue behaviour. #396

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

AlexJF
Copy link

@AlexJF AlexJF commented Mar 9, 2018

  • Payload queue could become stuck if queued payloads all expired due to max age before next flush (queue would become of size 0, so we'd never leave the queue state but we'd also never schedule a new retry).
  • Clean old (bigger than max age) payloads when inserting new ones not just on flush.
  • When we can't serialize a trace or service payload, we should reset our buffers because otherwise we'll probably end up in a serialization error loop and no progress will be made.
  • Fixed a flaky test in service writer after logic changes to the buffering mechanism.

* Payload queue could become stuck if queued payloads all expired due
  to max age before next flush (queue would become of size 0, so we'd
  never leave the queue state but we'd also never schedule a new retry).
* When we can't serialize a trace or service payload, we should reset
  our buffers because otherwise we'll probably end up in a serialization
  error loop and no progress will be made.
* Fixed a flaky test in service writer after logic changes to the
  buffering mechanism.
Copy link

@LotharSee LotharSee left a comment

Choose a reason for hiding this comment

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

Good catches! LGTM.

Copy link

@bmermet bmermet left a comment

Choose a reason for hiding this comment

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

Great catch! That was a tricky bug :)

@AlexJF AlexJF merged commit a974d0e into master Mar 12, 2018
@AlexJF AlexJF deleted the alexjf/fix-hanged-queue branch March 12, 2018 15:07
@palazzem palazzem added the core label Mar 19, 2018
@palazzem palazzem added this to the 6.1.0 milestone Mar 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants