-
Notifications
You must be signed in to change notification settings - Fork 308
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
feat: decode prom requests to grpc #3425
feat: decode prom requests to grpc #3425
Conversation
7588519
to
d631bdf
Compare
d631bdf
to
97c99b2
Compare
97c99b2
to
36435ae
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3425 +/- ##
==========================================
- Coverage 85.08% 84.49% -0.59%
==========================================
Files 889 893 +4
Lines 146488 146726 +238
==========================================
- Hits 124632 123978 -654
- Misses 21856 22748 +892 |
706c733
to
53669ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most LGTM
For performance reasons, we expect requests from the remote write protocol to be valid protobuf bytes. The protobuf standard says a string must always contain UTF-8 encoded or 7-bit ASCII text. However, we should provide a strict mode to perform UTF-8 validation or add this validation back once the string validation doesn't hurt the throughput too much. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! LGTM
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
A follow-up in #3478 to improve ~40% more.
What's changed and what's your intention?
This PR decodes
WriteRequest
in prometheus protocol directly to GreptimeDB'sRowInsertRequest
to avoid the overhead of conversions.The benchmark results show that it can reduce 32% deserialization time, hence a 47% improvement in throughput.
This PR does not involves object pooling since it does little to decode performance but brings complexity in object lifecycle management. Memory pooling may help in this case instead.
Checklist