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

fix: batch BQ writes by entire PRW request #334

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

karl-nilsson
Copy link
Contributor

Pull Request Template

Description

Batch the entire Remote-Write request, rather than per-timeseries. This allows a significantly higher write throughput.

Move the tagsFromMetric call out of the inner loop, it's only necessary to invoke it once per timeseries.

Type of change

  • New feature (non-breaking change which adds functionality)

Environment

  • Runtime version(Java, Go, Python, etc): Go v1.21

Batch the entire Remote-Write request, rather than per-timeseries. This
allows a significantly higher write throughput.

Also, move the tagsFromMetric call out of the inner loop.
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.00%. Comparing base (35d47e2) to head (e9ae4ce).

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #334       +/-   ##
===========================================
- Coverage   70.72%   40.00%   -30.73%     
===========================================
  Files           2        1        -1     
  Lines         263        5      -258     
===========================================
- Hits          186        2      -184     
+ Misses         66        3       -63     
+ Partials       11        0       -11     
Flag Coverage Δ
e2e ?
unit 40.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vinny-sabatini
Copy link
Member

Hi @karl-nilsson ! Unfortunately, the tests are failing because of #204 and I have not had time to try to fix our tests so they can be run from a fork. I pulled the code from your PR and ran the tests manually, they all passed and the changes look good.

Since I don't have time to fix #204 soon, I am okay with accepting the changes without fixing the pipeline, and I will try to create a release today. Thank you very much for your contribution!

@vinny-sabatini vinny-sabatini merged commit 227500d into KohlsTechnology:master Mar 8, 2024
3 of 4 checks passed
@vinny-sabatini
Copy link
Member

@karl-nilsson this has been included in v0.8.0, thanks again for your contribution!

@karl-nilsson karl-nilsson deleted the fixes branch March 9, 2024 02:47
@karl-nilsson
Copy link
Contributor Author

TYVM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants