diff options
author | John Cai <jcai@gitlab.com> | 2022-04-19 23:57:02 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-04-19 23:57:02 +0300 |
commit | d0809beb86fb02142c6f0d689929c08fd005b3fd (patch) | |
tree | f74de9a2d953e29af274d1b3a3845ae0d249a678 | |
parent | 752243b877d6b99feff18a601744635b80bd2b88 (diff) | |
parent | 6b2ab5c33ae05407170cee3e1b0bc5ec9fafa122 (diff) |
Merge branch 'jc-list-repo-to-use-shorter-grace-period' into 'master'
repocleaner: Allow NewWalker to receive grace period parameter
Closes #4164
See merge request gitlab-org/gitaly!4474
-rw-r--r-- | cmd/praefect/subcmd_list_untracked_repositories.go | 15 | ||||
-rw-r--r-- | cmd/praefect/subcmd_list_untracked_repositories_test.go | 46 | ||||
-rw-r--r-- | internal/praefect/repocleaner/repository.go | 13 |
3 files changed, 42 insertions, 32 deletions
diff --git a/cmd/praefect/subcmd_list_untracked_repositories.go b/cmd/praefect/subcmd_list_untracked_repositories.go index 39d6cd41c..dd6563f24 100644 --- a/cmd/praefect/subcmd_list_untracked_repositories.go +++ b/cmd/praefect/subcmd_list_untracked_repositories.go @@ -26,9 +26,10 @@ const ( var errNoConnectionToGitalies = errors.New("no connection established to gitaly nodes") type listUntrackedRepositories struct { - logger logrus.FieldLogger - delimiter string - out io.Writer + logger logrus.FieldLogger + onlyIncludeOlderThan time.Duration + delimiter string + out io.Writer } func newListUntrackedRepositories(logger logrus.FieldLogger, out io.Writer) *listUntrackedRepositories { @@ -38,6 +39,12 @@ func newListUntrackedRepositories(logger logrus.FieldLogger, out io.Writer) *lis func (cmd *listUntrackedRepositories) FlagSet() *flag.FlagSet { fs := flag.NewFlagSet(listUntrackedRepositoriesName, flag.ExitOnError) fs.StringVar(&cmd.delimiter, "delimiter", "\n", "string used as a delimiter in output") + fs.DurationVar( + &cmd.onlyIncludeOlderThan, + "older-than", + 6*time.Hour, + "only include repositories created before this duration", + ) fs.Usage = func() { printfErr("Description:\n" + " This command checks if all repositories on all gitaly nodes tracked by praefect.\n" + @@ -81,7 +88,7 @@ func (cmd listUntrackedRepositories) Exec(flags *flag.FlagSet, cfg config.Config defer func() { _ = db.Close() }() logger.Debug("connected to praefect database") - walker := repocleaner.NewWalker(nodeSet.Connections(), 16) + walker := repocleaner.NewWalker(nodeSet.Connections(), 16, cmd.onlyIncludeOlderThan) reporter := reportUntrackedRepositories{ ctx: ctx, checker: datastore.NewStorageCleanup(db), diff --git a/cmd/praefect/subcmd_list_untracked_repositories_test.go b/cmd/praefect/subcmd_list_untracked_repositories_test.go index a2c251a81..aa6fb8848 100644 --- a/cmd/praefect/subcmd_list_untracked_repositories_test.go +++ b/cmd/praefect/subcmd_list_untracked_repositories_test.go @@ -3,7 +3,6 @@ package main import ( "bytes" "context" - "flag" "fmt" "os" "strings" @@ -31,20 +30,20 @@ func TestListUntrackedRepositories_FlagSet(t *testing.T) { exp []interface{} }{ { - desc: "custom value", - args: []string{"--delimiter", ","}, - exp: []interface{}{","}, + desc: "custom values", + args: []string{"--delimiter", ",", "--older-than", "1s"}, + exp: []interface{}{",", time.Second}, }, { - desc: "default value", + desc: "default values", args: nil, - exp: []interface{}{"\n"}, + exp: []interface{}{"\n", 6 * time.Hour}, }, } { t.Run(tc.desc, func(t *testing.T) { fs := cmd.FlagSet() require.NoError(t, fs.Parse(tc.args)) - require.ElementsMatch(t, tc.exp, []interface{}{cmd.delimiter}) + require.ElementsMatch(t, tc.exp, []interface{}{cmd.delimiter, cmd.onlyIncludeOlderThan}) }) } } @@ -54,20 +53,6 @@ func TestListUntrackedRepositories_Exec(t *testing.T) { g1Cfg := testcfg.Build(t, testcfg.WithStorages("gitaly-1")) g2Cfg := testcfg.Build(t, testcfg.WithStorages("gitaly-2")) - // Repositories not managed by praefect. - repo1, repo1Path := gittest.InitRepo(t, g1Cfg, g1Cfg.Storages[0]) - repo2, repo2Path := gittest.InitRepo(t, g1Cfg, g1Cfg.Storages[0]) - _, _ = gittest.InitRepo(t, g2Cfg, g2Cfg.Storages[0]) - - require.NoError(t, os.Chtimes( - repo1Path, - time.Now().Add(-(24*time.Hour+1*time.Second)), - time.Now().Add(-(24*time.Hour+1*time.Second)))) - require.NoError(t, os.Chtimes( - repo2Path, - time.Now().Add(-(24*time.Hour+1*time.Second)), - time.Now().Add(-(24*time.Hour+1*time.Second)))) - g1Addr := testserver.RunGitalyServer(t, g1Cfg, nil, setup.RegisterAll, testserver.WithDisablePraefect()) g2Addr := testserver.RunGitalyServer(t, g2Cfg, nil, setup.RegisterAll, testserver.WithDisablePraefect()) @@ -104,7 +89,24 @@ func TestListUntrackedRepositories_Exec(t *testing.T) { createRepo(t, ctx, repoClient, praefectStorage, "path/to/test/repo") out := &bytes.Buffer{} cmd := newListUntrackedRepositories(testhelper.NewDiscardingLogger(t), out) - require.NoError(t, cmd.Exec(flag.NewFlagSet("", flag.PanicOnError), conf)) + fs := cmd.FlagSet() + require.NoError(t, fs.Parse([]string{})) + + // Repositories not managed by praefect. + repo1, repo1Path := gittest.InitRepo(t, g1Cfg, g1Cfg.Storages[0]) + repo2, repo2Path := gittest.InitRepo(t, g1Cfg, g1Cfg.Storages[0]) + _, _ = gittest.InitRepo(t, g2Cfg, g2Cfg.Storages[0]) + + require.NoError(t, os.Chtimes( + repo1Path, + time.Now().Add(-(6*time.Hour+1*time.Second)), + time.Now().Add(-(6*time.Hour+1*time.Second)))) + require.NoError(t, os.Chtimes( + repo2Path, + time.Now().Add(-(6*time.Hour+1*time.Second)), + time.Now().Add(-(6*time.Hour+1*time.Second)))) + + require.NoError(t, cmd.Exec(fs, conf)) exp := []string{ "The following repositories were found on disk, but missing from the tracking database:", diff --git a/internal/praefect/repocleaner/repository.go b/internal/praefect/repocleaner/repository.go index aeff3ac7d..afc8fd317 100644 --- a/internal/praefect/repocleaner/repository.go +++ b/internal/praefect/repocleaner/repository.go @@ -65,7 +65,7 @@ func NewRunner(cfg Cfg, logger logrus.FieldLogger, healthChecker praefect.Health logger: logger.WithField("component", "repocleaner.repository_existence"), healthChecker: healthChecker, conns: conns, - walker: NewWalker(conns, cfg.RepositoriesInBatch), + walker: NewWalker(conns, cfg.RepositoriesInBatch, 24*time.Hour), stateOwner: stateOwner, acquirer: acquirer, action: action, @@ -155,13 +155,14 @@ func (gs *Runner) loggerWith(virtualStorage, storage string) logrus.FieldLogger // Walker allows walk by the repositories of the gitaly storage. type Walker struct { - conns praefect.Connections - batchSize int + conns praefect.Connections + gracePeriod time.Duration + batchSize int } // NewWalker returns a new *Walker instance. -func NewWalker(conns praefect.Connections, batchSize int) *Walker { - return &Walker{conns: conns, batchSize: batchSize} +func NewWalker(conns praefect.Connections, batchSize int, gracePeriod time.Duration) *Walker { + return &Walker{conns: conns, batchSize: batchSize, gracePeriod: gracePeriod} } // ExecOnRepositories runs through all the repositories on a Gitaly storage and executes the provided action. @@ -189,7 +190,7 @@ func (wr *Walker) ExecOnRepositories(ctx context.Context, virtualStorage, storag // repositories that are in the process of being created, where // they do not yet have a record in Praefect. - if res.GetModificationTime().AsTime().After(time.Now().Add(-24 * time.Hour)) { + if res.GetModificationTime().AsTime().After(time.Now().Add(-wr.gracePeriod)) { continue } |