Skip to content

Create db indexing for upstream ref#1043

Open
kushnaidu wants to merge 5 commits into
kptdev:mainfrom
Nordix:upstream-db-index
Open

Create db indexing for upstream ref#1043
kushnaidu wants to merge 5 commits into
kptdev:mainfrom
Nordix:upstream-db-index

Conversation

@kushnaidu

@kushnaidu kushnaidu commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Title

Create db indexing for upstream ref, so that, looking up for references is quick.


Description

  • What changed:
  1. Added a new upstream_ref_name column to the package_revisions table that stores the upstream package revision name extracted from clone/upgrade tasks.
  2. Replaced the regex-based query in findUpstreamRefsFromDB (which scanned tasks::text with a pattern match) with a simple equality check on the new indexed column.
  3. Added a backfillUpstreamRefName function that runs on startup to populate the column for existing rows.
  4. The backfill code now uses smaller transactions with limits to avoid consuming too much memory on the system at startup if a lot of DB rows require an update.
  5. Added 5 B-tree indexes covering query paths: upstream ref lookup, revision ordering, lifecycle filtering, latest-revision lookup, and per-repo package listing.
  6. Add a mock.maybe for a flaky unit test because of watcher go routine.
  • Why it’s needed: The previous findUpstreamRefsFromDB used a PostgreSQL regex operator (~) against the full tasks text column, which forces a sequential scan on every call — no index can accelerate regex matches over arbitrary JSON text. This query runs on every package revision delete to check for downstream references, so it becomes a bottleneck as the number of package revisions grows.

  • How it works:
    On write/update, extractUpstreamRefName inspects the task list: for clone tasks it reads task.Clone.Upstream.UpstreamRef.Name, for upgrade tasks it reads task.Upgrade.NewUpstream.Name. The result is stored in upstream_ref_name.


Related Issue(s)

  • Closes/Fixes #

Type of Change

  • Bug fix
  • New feature
  • Enhancement
  • Refactor
  • Documentation
  • Tests
  • Other: ________

Checklist

  • Code follows project style guidelines
  • Self-reviewed changes
  • Tests added/updated
  • Documentation added/updated
  • All tests and gating checks pass

Testing Instructions (Optional)


Additional Notes (Optional)

  • Known issues:
  • Further improvements:
  • Review notes:

AI Disclosure

  • I have used AI in the creation of this PR.

If so, please describe how:
Microsoft Copilot to analyse the code.
Kiro to generate unit tests and DB backfill code.

Copilot AI review requested due to automatic review settings June 11, 2026 14:33
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 11, 2026
@netlify

netlify Bot commented Jun 11, 2026

Copy link
Copy Markdown

Deploy Preview for kpt-porch ready!

Name Link
🔨 Latest commit 7f1b9b9
🔍 Latest deploy log https://app.netlify.com/projects/kpt-porch/deploys/6a328f4312dc0300083dcb18
😎 Deploy Preview https://deploy-preview-1043--kpt-porch.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@kushnaidu kushnaidu force-pushed the upstream-db-index branch from f3efe9b to ec228d2 Compare June 11, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds a persisted upstream_ref_name to package_revisions so upstream-reference lookups can use an indexed column instead of regex matching against tasks.

Changes:

  • Add upstream_ref_name column and an index to support fast upstream reference lookups.
  • Populate upstream_ref_name on write/update and switch findUpstreamRefsFromDB to query the new column.
  • Add a startup backfill to populate upstream_ref_name for existing rows.

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pkg/cache/dbcache/dbpackagerevisionsql.go Writes/updates the new column, replaces regex search with indexed lookup, and adds a backfill routine.
pkg/cache/dbcache/dbcachefactory.go Runs the new upstream_ref_name backfill during cache initialization.
deployments/porch/3-porch-postgres-bundle.yaml Updates bundled Postgres schema with the new column and index.
api/sql/porch-db.sql Updates canonical schema with the new column and index.
api/sql/porch-db-1.6.0-1.5.9.sql Alters downgrade migration behavior for the new column.
api/sql/porch-db-1.5.11-1.6.0.sql Adds upgrade migration for the new column and related indexes.

Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread api/sql/porch-db-1.5.11-1.6.0.sql
Comment thread api/sql/porch-db-1.5.11-1.6.0.sql
Copilot AI review requested due to automatic review settings June 11, 2026 16:01
@kushnaidu kushnaidu force-pushed the upstream-db-index branch from 7394cf4 to 983adac Compare June 11, 2026 16:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.

Comment thread api/sql/porch-db-1.5.11-1.6.0.sql
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Copilot AI review requested due to automatic review settings June 12, 2026 11:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.

Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
@kushnaidu kushnaidu self-assigned this Jun 15, 2026
Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
@kushnaidu kushnaidu force-pushed the upstream-db-index branch from c6f5b72 to 6f3c16f Compare June 16, 2026 09:59
Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>
Copilot AI review requested due to automatic review settings June 17, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.

Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
Comment thread pkg/cache/dbcache/dbpackagerevisionsql.go Outdated
@sonarqubecloud

Copy link
Copy Markdown

Signed-off-by: Kushal Harish Naidu <kushal.harish.naidu@ericsson.com>

@kushnaidu kushnaidu left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@kushnaidu kushnaidu removed the do-not-merge/hold #ededed label Jun 18, 2026
@dosubot dosubot Bot added the lgtm #ededed label Jun 18, 2026
@liamfallon liamfallon self-requested a review June 18, 2026 08:24
@nagygergo

Copy link
Copy Markdown
Contributor

Why does the backfill methods need to be broken down to multiple transactions?

@kushnaidu

Copy link
Copy Markdown
Contributor Author

Why does the backfill methods need to be broken down to multiple transactions?

Copilot suggested it so that there won't be sudden spike at the start of the system pod when there are 1000s of entries which require an update.

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

Labels

lgtm #ededed size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants