Skip to content

Conversation

@Emyrk
Copy link
Member

@Emyrk Emyrk commented Nov 21, 2025

closes #19889

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

Comment on lines 89 to 90
// TODO: Arbitrary numbers are arbitrary...
LimitCount: 10000,
Copy link
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
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
Copy link
Member Author

Emyrk commented Nov 21, 2025

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;

Copy link
Member Author

Emyrk commented Nov 21, 2025

@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
@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
@Emyrk Emyrk merged commit cefe07d into main Nov 24, 2025
31 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

3 participants