diff options
author | Karthik Nayak <knayak@gitlab.com> | 2023-06-02 13:00:55 +0300 |
---|---|---|
committer | Karthik Nayak <knayak@gitlab.com> | 2023-06-05 11:04:56 +0300 |
commit | 2b9016b83888a05e34ae0bdd5afb6cdd69d92dff (patch) | |
tree | 0307a73464180515b09b44bff8331d5f83b5d814 | |
parent | cfd146b4d96acd8f1cb5ca06694e8631dff51368 (diff) |
housekeeping: Allow context cancellation of git-pack-refs(1)5316-check-metrics-and-decide-if-need-to-context-cancel-the-running-git-process-in
We can add inhibitors from running git-pack-refs(1) via the
`addPackRefsInhibitor()` function. But if there is a ongoing
git-pack-refs(1), we are blocked until it finishes.
We instead use a context and propagate cancellation via the context.
This allows the inhibitors to take priority and not be blocked over
git-pack-refs(1).
-rw-r--r-- | internal/git/housekeeping/manager.go | 19 | ||||
-rw-r--r-- | internal/git/housekeeping/optimize_repository.go | 2 | ||||
-rw-r--r-- | internal/git/housekeeping/optimize_repository_test.go | 6 |
3 files changed, 22 insertions, 5 deletions
diff --git a/internal/git/housekeeping/manager.go b/internal/git/housekeeping/manager.go index d73152182..c7804432a 100644 --- a/internal/git/housekeeping/manager.go +++ b/internal/git/housekeeping/manager.go @@ -32,6 +32,9 @@ type repositoryState struct { // packRefsDone is a channel used to denote when the ongoing (if any) call to packRefsIfNeeded // is completed. packRefsDone chan struct{} + // packRefsCancel is the context cancellation function which can be used to cancel any + // running git-pack-refs(1). + packRefsCancel context.CancelFunc // packRefsInhibitors keeps a count of the number of inhibitors on running packRefsIfNeeded. packRefsInhibitors int32 // isRunning is used to indicate if housekeeping is running. @@ -139,6 +142,12 @@ func (s *repositoryStates) addPackRefsInhibitor(ctx context.Context, repoPath st break } + // Instead of waiting for git-pack-refs to finish, we ask it to + // stop. + if state.packRefsCancel != nil { + state.packRefsCancel() + } + state.Unlock() select { @@ -174,7 +183,7 @@ func (s *repositoryStates) addPackRefsInhibitor(ctx context.Context, repoPath st // is at least one inhibitors then we return false. If there are no inhibitors, we setup the `packRefsDone` // channel to denote when `git-pack-refs(1)` finishes, this is handled when the caller calls the // cleanup function returned by this function. -func (s *repositoryStates) tryRunningPackRefs(repoPath string) (successful bool, _ func()) { +func (s *repositoryStates) tryRunningPackRefs(ctx context.Context, repoPath string) (successful bool, _ context.Context, _ func()) { state, cleanup := s.getState(repoPath) defer func() { if !successful { @@ -186,12 +195,15 @@ func (s *repositoryStates) tryRunningPackRefs(repoPath string) (successful bool, defer state.Unlock() if state.packRefsInhibitors > 0 || state.packRefsDone != nil { - return false, nil + return false, ctx, nil } state.packRefsDone = make(chan struct{}) - return true, func() { + ctx, cancel := context.WithCancel(ctx) + state.packRefsCancel = cancel + + return true, ctx, func() { defer cleanup() state.Lock() @@ -199,6 +211,7 @@ func (s *repositoryStates) tryRunningPackRefs(repoPath string) (successful bool, close(state.packRefsDone) state.packRefsDone = nil + state.packRefsCancel = nil } } diff --git a/internal/git/housekeeping/optimize_repository.go b/internal/git/housekeeping/optimize_repository.go index ec010d33a..af732e138 100644 --- a/internal/git/housekeeping/optimize_repository.go +++ b/internal/git/housekeeping/optimize_repository.go @@ -287,7 +287,7 @@ func (m *RepositoryManager) packRefsIfNeeded(ctx context.Context, repo *localrep } // If there are any inhibitors, we don't run git-pack-refs(1). - ok, cleanup := m.repositoryStates.tryRunningPackRefs(path) + ok, ctx, cleanup := m.repositoryStates.tryRunningPackRefs(ctx, path) if !ok { return false, nil } diff --git a/internal/git/housekeeping/optimize_repository_test.go b/internal/git/housekeeping/optimize_repository_test.go index 419e039ba..36815c73d 100644 --- a/internal/git/housekeeping/optimize_repository_test.go +++ b/internal/git/housekeeping/optimize_repository_test.go @@ -389,10 +389,14 @@ func TestPackRefsIfNeeded(t *testing.T) { b.block["pack-refs"] = ch return setupData{ - refsShouldBePacked: true, + // addPackRefsInhibitor cancels the context of the git-pack-refs(1) + refsShouldBePacked: false, shouldPackReferences: func(_ context.Context) bool { return true }, + errExpected: fmt.Errorf("packing refs: %w, stderr: %q", + fmt.Errorf("getting Git version: %w", + fmt.Errorf("spawning version command: %w", context.Canceled)), ""), } }, }, |