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

[server] Fix stats handling for SINGLE_GET requests #1381

Merged

Conversation

nisargthakkar
Copy link
Contributor

@nisargthakkar nisargthakkar commented Dec 10, 2024

Fix stats handling for SINGLE_GET requests

In a previous commit (#1377), when removing some metrics, we ended up removing some key metrics about key and value size values for single_get requests as well. That change also led to NPEs as the caller expected the K/V stats to always be recorded for Single-get requests. This commit adds Avg and Max stats by default for key and value sizes in single get requests.

How was this PR tested?

Added unit tests

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

@nisargthakkar nisargthakkar force-pushed the fixMissingMetricsForSingleGet branch from 87cfcdd to 8d6b2a9 Compare December 10, 2024 21:13
@nisargthakkar nisargthakkar marked this pull request as ready for review December 10, 2024 21:32
@nisargthakkar nisargthakkar force-pushed the fixMissingMetricsForSingleGet branch from 701ce46 to b98e153 Compare December 10, 2024 21:33
@nisargthakkar nisargthakkar enabled auto-merge (squash) December 10, 2024 21:34
Copy link
Collaborator

@sushantmane sushantmane left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @nisargthakkar!

@nisargthakkar nisargthakkar merged commit df62060 into linkedin:main Dec 10, 2024
51 checks passed
@nisargthakkar nisargthakkar deleted the fixMissingMetricsForSingleGet branch December 10, 2024 22:37
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.

2 participants