Skip to content

Conversation

@ggevay
Copy link
Contributor

@ggevay ggevay commented Jun 5, 2025

Add histogram metric for time between first result row returned and last result row returned. (This is part of #32504, although it's not yet mentioned in the design doc.)

This is more involved than my other similar PRs from yesterday, because the control flow for returning results is a bit complex, with several variations in various situations, see tests. (I wanted to also add a test for a FETCH that has a timeout, but it seems FETCH's timeout is currently not working: https://github.com/MaterializeInc/database-issues/issues/9354 )

Nightly: https://buildkite.com/materialize/nightly/builds/12231

Motivation

  • This PR adds a feature that has not yet been specified.

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@ggevay ggevay force-pushed the first-last-byte-returned-metric branch 3 times, most recently from 4dc8fd8 to 94324d6 Compare June 5, 2025 20:06
0.000_008, 0.000_016, 0.000_032, 0.000_064, 0.000_128, 0.000_256, 0.000_512, 0.001, 0.002,
0.004, 0.008, 0.016, 0.032, 0.064, 0.128, 0.256, 0.512, 1.0, 2.0, 4.0, 8.0, 16.0, 32.0,
0.004, 0.008, 0.016, 0.032, 0.064, 0.128, 0.256, 0.512, 1.0, 2.0, 4.0, 8.0, 16.0, 32.0, 64.0,
128.0, 256.0, 512.0, 1024.0, 2048.0, 4096.0, 8192.0,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I've checked that the only existing metric that this change affected is one of the metrics that I added yesterday: mz_pgwire_message_processing_seconds. I didn't know that there is this limitation when I added that metric, so what I put into the histogram_seconds_buckets was not taking effect. Now this is fixed.)

@ggevay ggevay marked this pull request as ready for review June 5, 2025 20:14
@ggevay ggevay requested a review from a team as a code owner June 5, 2025 20:14
@ggevay ggevay requested a review from aljoscha June 5, 2025 20:14
@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Jun 5, 2025
@ggevay ggevay force-pushed the first-last-byte-returned-metric branch from 94324d6 to 1e788c7 Compare June 17, 2025 14:06
@ggevay ggevay force-pushed the first-last-byte-returned-metric branch from 1e788c7 to de38ba0 Compare June 18, 2025 14:54
@ggevay ggevay enabled auto-merge June 18, 2025 14:54
@ggevay ggevay merged commit 2c37722 into MaterializeInc:main Jun 18, 2025
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ADAPTER Topics related to the ADAPTER layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants