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(http): large prometheus metrics responses split across multiple chunks #3843

Merged
merged 36 commits into from Oct 26, 2023

Conversation

amunra
Copy link
Contributor

@amunra amunra commented Oct 13, 2023

Change

This is a minor fix for the way that we handle prometheus metrics.
Prometheus metrics are exposed as the /metrics URL on the http-min port.
Currently we don't report enough metrics to cause a problem, but if we were to expand the amount of metrics we produce we'd run out of the fixed buffer size (defaults to 1MiB).

The buffer size is fixed, because we return responeses with the "chunked" transfer encoding (HTTP 1.1).

This PR also includes test enhancements that perform parallel requests to the Prometheus endpoint.

Implementation

This fix first encodes all the metrics to a separate buffer, then copies that buffer to as many chunk as required.

The serialization buffer, along with a written counter tracking how many bytes got copied to http chunks is tracked inside RequestState objects. To avoid continuously calling malloc and free for potentially larger buffers, these objects are pooled and reused.

The pool is shared across all instances of the PrometheusMetricsProcessor (one per worker).
This avoids allocating multiple large buffers in case of sparse requests (which is the default use case for the /metrics URL).

Additionally, the Scrapable metrics interface has been updated from using CharSink to DirectUtf8CharSink which allows borrowing the writable buffer as a C questdb_byte_sink_t* and adding metrics from native code.

@amunra amunra changed the title fix(http): Chunked response metrics fix(http): large metrics responses can be split across multiple chunks Oct 13, 2023
@amunra amunra changed the title fix(http): large metrics responses can be split across multiple chunks fix(http): large prometheus metrics responses split across multiple chunks Oct 13, 2023
@amunra amunra marked this pull request as ready for review October 13, 2023 19:58
@amunra amunra marked this pull request as draft October 16, 2023 09:48
@amunra amunra marked this pull request as ready for review October 16, 2023 12:21
@amunra
Copy link
Contributor Author

amunra commented Oct 20, 2023

Merged latest changes. This PR is ready to be reviewed again.

@amunra amunra marked this pull request as draft October 20, 2023 15:33
@amunra amunra marked this pull request as ready for review October 25, 2023 14:17
ideoma
ideoma previously approved these changes Oct 26, 2023
bluestreak01
bluestreak01 previously approved these changes Oct 26, 2023
@ideoma
Copy link
Collaborator

ideoma commented Oct 26, 2023

[PR Coverage check]

😍 pass : 80 / 80 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/cutlass/http/HttpResponseSink.java 5 5 100.00%
🔵 io/questdb/cutlass/http/HttpServer.java 4 4 100.00%
🔵 io/questdb/std/str/DirectByteCharSink.java 5 5 100.00%
🔵 io/questdb/cutlass/Services.java 4 4 100.00%
🔵 io/questdb/cutlass/http/processors/PrometheusMetricsProcessor.java 62 62 100.00%

@bluestreak01 bluestreak01 merged commit bd92d9b into master Oct 26, 2023
18 of 21 checks passed
@bluestreak01 bluestreak01 deleted the chunked_response_metrics branch October 26, 2023 17:38
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.

None yet

3 participants