Skip to content

feat: purge expired api keys in dbpurge - #20863

Merged
Emyrk merged 10 commits into
mainfrom
stevenmasley/purge_expired_keys
Nov 24, 2025
Merged

Emyrk merged 10 commits into
mainfrom
stevenmasley/purge_expired_keys

Conversation

@Emyrk

@Emyrk Emyrk commented Nov 21, 2025 •

Copy link
Copy Markdown
Member

closes #19889

This is in response to a migration in v2.27 that takes very long on deployments with large api_key tables.

Comment thread coderd/database/dbpurge/dbpurge.go Outdated
Comment on lines +89 to +90
// TODO: Arbitrary numbers are arbitrary...
LimitCount: 10000,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We know an api_key migration takes so long it can timeout.

I don't like arbitrary numbers, but if this table is massive, we should spread the load out. DB purge runs every 10 minutes. 10k feels large, but not insane to be too slow?

Comment on lines +82 to +86
// Leave expired keys for a week to allow the backend to know the difference
// between a 404 and an expired key. This purge code is just to bound the size of
// the table to something more reasonable.
// TODO: Does this matter?
Before: dbtime.Time(start.Add(time.Hour * 24 * 7 * -1)),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is a behavior change if we purge expired keys. And I wonder if the difference between 404 & expired is good information.

@Emyrk

Emyrk commented Nov 21, 2025 •

Copy link
Copy Markdown
Member Author

oauth2_provider_app_tokens references api_keys. I think we can't just delete without deleting the app_token too. Will write a test and check

We are good. The oauth token is also deleted from the DELETE CASCADE

ALTER TABLE ONLY oauth2_provider_app_tokens
    ADD CONSTRAINT oauth2_provider_app_tokens_api_key_id_fkey FOREIGN KEY (api_key_id) REFERENCES api_keys(id) ON DELETE CASCADE;

Emyrk commented Nov 21, 2025 •

Copy link
Copy Markdown
Member Author

@Emyrk
Emyrk force-pushed the stevenmasley/purge_expired_keys branch from ceb15c5 to 11bf986 Compare November 21, 2025 16:29
@Emyrk
Emyrk marked this pull request as ready for review November 21, 2025 16:35
Comment thread coderd/database/queries/apikeys.sql Resolved
Comment thread coderd/database/dbpurge/dbpurge.go Resolved
@Emyrk
Emyrk force-pushed the stevenmasley/purge_expired_keys branch from 0cd1c02 to 43290cd Compare November 21, 2025 17:55
@Emyrk
Emyrk requested a review from dannykopping November 21, 2025 18:18
Comment thread coderd/database/dbpurge/dbpurge.go Resolved
Comment thread coderd/database/dbpurge/dbpurge.go Resolved
@Emyrk
Emyrk merged commit cefe07d into main Nov 24, 2025
26 checks passed
@Emyrk
Emyrk deleted the stevenmasley/purge_expired_keys branch November 24, 2025 16:24
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 24, 2025
@Emyrk Emyrk added cherry-pick/v2.29 Needs to be cherry-picked to the 2.29 release branch and removed cherry-pick/v2.29 Needs to be cherry-picked to the 2.29 release branch labels Dec 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dbpurge: purge expired api keys

2 participants