Skip to content

Conversation

@myrrc
Copy link
Contributor

@myrrc myrrc commented Apr 24, 2025

#10327
Resolves: #11720

New metrics:

  • compute_getpage_stuck_requests_total
  • compute_getpage_max_inflight_stuck_time_ms

@myrrc myrrc requested review from a team as code owners April 24, 2025 21:22
@myrrc myrrc requested review from bizwark, ololobus and skyzh April 24, 2025 21:22
@github-actions
Copy link

If this PR added a GUC in the Postgres fork or neon extension,
please regenerate the Postgres settings in the cloud repo:

make NEON_WORKDIR=path/to/neon/checkout \
  -C goapp/internal/shareddomain/postgres generate

If you're an external contributor, a Neon employee will assist in
making sure this step is done.

@github-actions
Copy link

github-actions bot commented Apr 24, 2025

8327 tests run: 7827 passed, 0 failed, 500 skipped (full report)


Flaky tests (2)

Postgres 17

Postgres 15

Code coverage* (full report)

  • functions: 32.7% (8958 of 27399 functions)
  • lines: 48.9% (78182 of 160031 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
56ab95c at 2025-04-28T18:43:20.006Z :recycle:

@myrrc myrrc force-pushed the myrrc/10327-stuck-getpage-metrics branch from 60142ea to 39a764a Compare April 25, 2025 09:19
@myrrc myrrc changed the title getpage_stuck_requests_total Postgres metric Postgres metrics for stuck getpage requests Apr 25, 2025
@myrrc myrrc requested a review from ololobus April 25, 2025 11:15
@myrrc myrrc requested a review from MMeent April 28, 2025 13:38
@myrrc myrrc requested a review from MMeent April 28, 2025 17:29
@myrrc myrrc force-pushed the myrrc/10327-stuck-getpage-metrics branch from b42bf49 to 56ab95c Compare April 28, 2025 17:33
Copy link
Contributor

@knizhnik knizhnik left a comment

Choose a reason for hiding this comment

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

I wonder why instead just calculating maximal time we can not just use histogram as for example file_cache_write_hist.
In this In this case we do not need separate counter metrics for stuck requests.

@myrrc
Copy link
Contributor Author

myrrc commented Apr 29, 2025

I've considered this as an option, but our current latency histogram treats 10s as +Inf, so either adding a new bucket or another metrics.

Or you suggest turning max_ metric into a histogram? That's also an option but seems a bit extra for me, as we won't care for any but max values, and most of the histogram would just duplicated the exiting histogram for latencies

@myrrc myrrc requested review from knizhnik and removed request for bizwark April 29, 2025 20:13
@myrrc myrrc added this pull request to the merge queue Apr 30, 2025
Merged via the queue into main with commit 8da4ec9 Apr 30, 2025
103 checks passed
@myrrc myrrc deleted the myrrc/10327-stuck-getpage-metrics branch April 30, 2025 12:03
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.

getpage stuck requests: metrics in postgres

6 participants