repo-updater: run repo purge on regular interval with generous TTL#44753
Conversation
a2c4dc4 to
63ba56a
Compare
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 12be752...b1da12c.
|
63ba56a to
59336df
Compare
There was a problem hiding this comment.
Nice LGTM 👍 .
I'm speaking from just gut feeling but I feel like 30 min is a bit short for a grace period, how would you feel about an hour? That way there's more of a chance that if someone does a config change that removes repos and doesn't realize it, there's more of a chance that someone else might notice and alert it 😅 .
| // gitserver, but not enabled/present in our repos table. ttl, should be >= 0 and | ||
| // specifies how long ago a repo must be deleted before it is purged. | ||
| func RunRepositoryPurgeWorker(ctx context.Context, logger log.Logger, db database.DB, ttl time.Duration) { | ||
| func RunRepositoryPurgeWorker(ctx context.Context, logger log.Logger, db database.DB, ttl time.Duration, baseInterval time.Duration) { |
There was a problem hiding this comment.
do we want to make this an operation (so we can have traces and stuff)? not sure how valuable it would be, but I suppose we want to know when we are purging repos
There was a problem hiding this comment.
I think for now we can rely on logs - we can add tracing in a followup
michaellzc
left a comment
There was a problem hiding this comment.
left some comments but :lgtm
SGTM, bumped to an hour, which will also now be configurable via https://github.com/sourcegraph/sourcegraph/pull/44753/commits/81ecfaa0f66d667771909c137951c691601df655 |
Co-authored-by: Michael Lin <mlzc@hey.com>
| // According to The Cure, 10:15 Saturday Night you should be sitting in your | ||
| // kitchen sink, not adjusting your external service configuration. | ||
| return t.Format("Mon 15") == "Sat 22" |
| "default": "15", | ||
| "minimum": 0 | ||
| }, | ||
| "deletedTTLMintues": { |
There was a problem hiding this comment.
Minutes not Mintues - that should be fixed, since it's user-visible
There was a problem hiding this comment.
doh - good catch, thank you! https://github.com/sourcegraph/sourcegraph/pull/44794
| "deletedTTL": 60 | ||
| }, | ||
| "properties": { | ||
| "intervalMintues": { |
There was a problem hiding this comment.
Minutes not Mintues - that should be fixed, since it's user-visible
We only clear deleted repos on weekends, documented as a stopgap to mitigate the impact of accidental deletions. This seems to cause large numbers of repositories to linger around if a massive code host is added and deleted, consuming disk space that the percentage-based janitor seems to struggle to keep up with - we saw this in S2 recently: thread
60-75 minutes seems a sufficiently generous amount of time to allow for a mistaken deletion to be reverted, so we remove the weekend constraint to allow this to run more frequently.
Test plan
CI, tested site configuration with
sg start