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.

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

Remove accidental legacy field, fix metrics #310

Merged
merged 1 commit into from
Dec 11, 2021

Conversation

hannahhoward
Copy link
Collaborator

Goals

Well, I guess this is the price you pay for not having testing on stats! we broke the request queue stats when we did the request queue refactor

Implementation

  • Remove peer task queue legacy struct member
  • Use responseQueue for stats instead
  • Also noticed we broke peer peer limits for the responder here.

@hannahhoward
Copy link
Collaborator Author

BTW, I would have added a test for the stats here but I'm really struggling to figure out where to run it so we have no race condition but get accurate values

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

Oh dear ..

@hannahhoward hannahhoward merged commit 22bb4bd into main Dec 11, 2021
hannahhoward added a commit that referenced this pull request Dec 11, 2021
fix(impl): remove accidental legacy field (#310)

feat: add basic tracing for responses (#291)

* feat: add basic tracing for responses

* fix: add assertions for response completion

* fix: improve test synchronization/drainage

* fix(responsemanager): remove uninformative span flags

Co-authored-by: hannahhoward <[email protected]>
@mvdan mvdan deleted the fix/remove-peer-task-queue branch December 15, 2021 14:17
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