Skip to content

fix(deployments): stop full scans and coalesce cache misses - #793

Open
romeroyonatan wants to merge 2 commits into
grafana:mainfrom
Shiphero:romeroyonatan/fix-deployments-full-scan
Open

fix(deployments): stop full scans and coalesce cache misses#793
romeroyonatan wants to merge 2 commits into
grafana:mainfrom
Shiphero:romeroyonatan/fix-deployments-full-scan

Conversation

@romeroyonatan

@romeroyonatan romeroyonatan commented Aug 5, 2026

Copy link
Copy Markdown

Issue

  1. Deployment range queries scanned every GitHub page resulting in slow deployments dashboards
  2. Concurrent cache misses duplicated requests:
panel A: cache miss → mutex unlock → GitHub request
panel B: cache miss → mutex unlock → GitHub request

Solution

  1. Paginate until deployments predate the requested range
  2. Coalesce identical deployment cache misses.

Implementation detail

singleflight.Group tracks in-flight work by cache key, so identical misses share one request. The field deploymentRequests singleflight.Group is deployment-specific because this PR intentionally changes only deployments

@cla-assistant

cla-assistant Bot commented Aug 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Aug 5, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread pkg/github/deployments.go
if createdAt == nil {
continue
}
if !createdAt.Before(from) && !createdAt.After(to) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

wouldn't createdAt.After(from) && createdAT.Before(to) be more idiomatic? also bear in mind that this is < not <= just in case

@romeroyonatan romeroyonatan Aug 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For this PR, no: the requirement is inclusive [from, to].

!createdAt.Before(from) && !createdAt.After(to)

means:

createdAt >= from && createdAt <= to

Using:

createdAt.After(from) && createdAt.Before(to)

would mean:

createdAt > from && createdAt < to

and would incorrectly exclude deployments exactly at from or to.

The current form is idiomatic for inclusive time.Time bounds because Go has no
BetweenInclusive helper.

This pattern is also present in other code in this repo. For example:

https://github.com/grafana/github-datasource/blob/main/pkg/github/workflows.go#L102

Comment thread pkg/github/deployments.go
Comment thread pkg/github/deployments.go Outdated
Issue
Deployment range queries scanned every GitHub page, and concurrent cache misses
duplicated scans.

Solution
Paginate until deployments predate the requested range and coalesce identical deployment cache misses.
@romeroyonatan
romeroyonatan force-pushed the romeroyonatan/fix-deployments-full-scan branch from fc835ce to 805fc18 Compare August 5, 2026 15:16
@romeroyonatan romeroyonatan changed the title fix(deployments): stop range scans and coalesce cache misses fix(deployments): stop full scans and coalesce cache misses Aug 5, 2026
@romeroyonatan
romeroyonatan marked this pull request as ready for review August 5, 2026 15:31
@romeroyonatan
romeroyonatan requested a review from a team as a code owner August 5, 2026 15:31
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.

2 participants