Skip to content

Fix a bug where lfs gc never worked. #35198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions modules/setting/cron_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,56 @@ EXTEND = true
assert.Equal(t, "white rabbit", extended.Second)
assert.True(t, extended.Extend)
}

// Test_getCronSettings2 tests that getCronSettings can not handle two levels of embedding
func Test_getCronSettings2(t *testing.T) {
type BaseStruct struct {
Enabled bool
RunAtStart bool
Schedule string
}

type Extended struct {
BaseStruct
Extend bool
}
type Extended2 struct {
Extended
Third string
}

iniStr := `
[cron.test]
ENABLED = TRUE
RUN_AT_START = TRUE
SCHEDULE = @every 1h
EXTEND = true
THIRD = white rabbit
`
cfg, err := NewConfigProviderFromData(iniStr)
assert.NoError(t, err)

extended := &Extended2{
Extended: Extended{
BaseStruct: BaseStruct{
Enabled: false,
RunAtStart: false,
Schedule: "@every 72h",
},
Extend: false,
},
Third: "black rabbit",
}

_, err = getCronSettings(cfg, "test", extended)
assert.NoError(t, err)

// This confirms the first level of embedding works
assert.Equal(t, "white rabbit", extended.Third)
assert.True(t, extended.Extend)

// This confirms 2 levels of embedding doesn't work
assert.False(t, extended.Enabled)
assert.False(t, extended.RunAtStart)
assert.Equal(t, "@every 72h", extended.Schedule)
}
43 changes: 22 additions & 21 deletions services/cron/tasks_extended.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,34 +171,35 @@ func registerDeleteOldSystemNotices() {
})
}

type GCLFSConfig struct {
BaseConfig
OlderThan time.Duration
LastUpdatedMoreThanAgo time.Duration
NumberToCheckPerRepo int64
ProportionToCheckPerRepo float64
}

func registerGCLFS() {
if !setting.LFS.StartServer {
return
}
type GCLFSConfig struct {
OlderThanConfig
LastUpdatedMoreThanAgo time.Duration
NumberToCheckPerRepo int64
ProportionToCheckPerRepo float64
}

RegisterTaskFatal("gc_lfs", &GCLFSConfig{
OlderThanConfig: OlderThanConfig{
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
// objects.
//
// It is likely that a week is potentially excessive but it should definitely be enough that any
// unassociated LFS object is genuinely unassociated.
OlderThan: 24 * time.Hour * 7,
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
// objects.
//
// It is likely that a week is potentially excessive but it should definitely be enough that any
// unassociated LFS object is genuinely unassociated.
OlderThan: 24 * time.Hour * 7,

// Only GC things that haven't been looked at in the past 3 days
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
NumberToCheckPerRepo: 100,
Expand Down
51 changes: 51 additions & 0 deletions services/cron/tasks_extended_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package cron

import (
"testing"
"time"

"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"

"github.com/stretchr/testify/assert"
)

func Test_GCLFSConfig(t *testing.T) {
cfg, err := setting.NewConfigProviderFromData(`
[cron.gc_lfs]
ENABLED = true
RUN_AT_START = true
SCHEDULE = "@every 2h"
OLDER_THAN = "1h"
LAST_UPDATED_MORE_THAN_AGO = "7h"
NUMBER_TO_CHECK_PER_REPO = 10
PROPORTION_TO_CHECK_PER_REPO = 0.1
`)
assert.NoError(t, err)
defer test.MockVariableValue(&setting.CfgProvider, cfg)()

config := &GCLFSConfig{
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
OlderThan: 24 * time.Hour * 7,
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
NumberToCheckPerRepo: 100,
ProportionToCheckPerRepo: 0.6,
}

_, err = setting.GetCronSettings("gc_lfs", config)
assert.NoError(t, err)
assert.True(t, config.Enabled)
assert.True(t, config.RunAtStart)
assert.Equal(t, "@every 2h", config.Schedule)
assert.Equal(t, 1*time.Hour, config.OlderThan)
assert.Equal(t, 7*time.Hour, config.LastUpdatedMoreThanAgo)
assert.Equal(t, int64(10), config.NumberToCheckPerRepo)
assert.InDelta(t, 0.1, config.ProportionToCheckPerRepo, 0.001)
}