diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index e0a8e181..a99bead5 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -415,13 +415,56 @@ func (r *Repository) MarkReady() { r.mu.Unlock() } +// Clone starts one mirror clone. Other callers wait for the result. func (r *Repository) Clone(ctx context.Context) error { + return r.clone(ctx, 0) +} + +// CloneWithWaitTimeout limits how long a caller waits for another clone. +// The limit does not apply to the caller that starts the clone. +func (r *Repository) CloneWithWaitTimeout(ctx context.Context, waitTimeout time.Duration) error { + return r.clone(ctx, waitTimeout) +} + +func (r *Repository) clone(ctx context.Context, waitTimeout time.Duration) error { + if r.TryStartCloning() { + return r.CloneClaimed(ctx) + } + if waitTimeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, waitTimeout) + defer cancel() + } + + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + for { + switch r.State() { + case StateReady: + return nil + case StateEmpty: + return errors.New("repository clone did not complete") + case StateCloning: + } + select { + case <-ctx.Done(): + return errors.Wrap(ctx.Err(), "wait for repository clone") + case <-ticker.C: + } + } +} + +// CloneClaimed requires the caller to acquire ownership with TryStartCloning first. +func (r *Repository) CloneClaimed(ctx context.Context) error { r.mu.Lock() if r.state == StateReady { r.mu.Unlock() return nil } - r.state = StateCloning + if r.state != StateCloning { + r.mu.Unlock() + return errors.New("repository clone was not claimed") + } r.mu.Unlock() err := r.executeClone(ctx) diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index c64fb6a8..1cfcf77d 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/alecthomas/assert/v2" + "github.com/alecthomas/errors" "github.com/block/cachew/internal/logging" ) @@ -415,6 +416,41 @@ func TestRepository_Clone_StateVisibleDuringClone(t *testing.T) { assert.Equal(t, StateReady, repo.State()) } +func TestRepositoryCloneWaitsForCurrentOwner(t *testing.T) { + repo := &Repository{ + state: StateCloning, + config: testRepoConfig(), + path: filepath.Join(t.TempDir(), "clone"), + upstreamURL: "https://example.test/example/repo", + fetchSem: make(chan struct{}, 1), + } + repo.fetchSem <- struct{}{} + ready := make(chan struct{}) + go func() { + time.Sleep(20 * time.Millisecond) + repo.MarkReady() + close(ready) + }() + + assert.NoError(t, repo.Clone(t.Context())) + <-ready + assert.Equal(t, StateReady, repo.State()) +} + +func TestRepositoryCloneWaitTimeout(t *testing.T) { + repo := &Repository{ + state: StateCloning, + config: testRepoConfig(), + path: filepath.Join(t.TempDir(), "clone"), + upstreamURL: "https://example.test/example/repo", + fetchSem: make(chan struct{}, 1), + } + repo.fetchSem <- struct{}{} + + err := repo.CloneWithWaitTimeout(t.Context(), 20*time.Millisecond) + assert.True(t, errors.Is(err, context.DeadlineExceeded)) +} + func TestRepository_CloneSetsMirrorConfig(t *testing.T) { ctx := context.Background() tmpDir := t.TempDir() diff --git a/internal/jobscheduler/jobs.go b/internal/jobscheduler/jobs.go index ceee18dd..36a7e892 100644 --- a/internal/jobscheduler/jobs.go +++ b/internal/jobscheduler/jobs.go @@ -48,6 +48,8 @@ type Scheduler interface { // // Jobs run concurrently across queues, but never within a queue. Submit(queue, id string, run func(ctx context.Context) error) + // TrySubmit adds a job only if fewer than maxQueued jobs wait across all queues. + TrySubmit(queue, id string, maxQueued int, run func(ctx context.Context) error) bool // SubmitPeriodicJob submits a job to the queue that runs immediately, and then periodically after the interval. // // Jobs run concurrently across queues, but never within a queue. @@ -63,6 +65,10 @@ func (p *prefixedScheduler) Submit(queue, id string, run func(ctx context.Contex p.scheduler.Submit(queue, p.prefix+id, run) } +func (p *prefixedScheduler) TrySubmit(queue, id string, maxQueued int, run func(ctx context.Context) error) bool { + return p.scheduler.TrySubmit(queue, p.prefix+id, maxQueued, run) +} + func (p *prefixedScheduler) SubmitPeriodicJob(queue, id string, interval time.Duration, run func(ctx context.Context) error) { p.scheduler.SubmitPeriodicJob(queue, p.prefix+id, interval, run) } @@ -190,6 +196,20 @@ func (q *RootScheduler) Submit(queue, id string, run func(ctx context.Context) e q.cond.Signal() } +// TrySubmit limits the queue size without a wait. It rejects new jobs during shutdown. +func (q *RootScheduler) TrySubmit(queue, id string, maxQueued int, run func(ctx context.Context) error) bool { + q.lock.Lock() + if q.done || q.draining || maxQueued <= 0 || len(q.queue) >= maxQueued { + q.lock.Unlock() + return false + } + q.queue = append(q.queue, queueJob{queue: queue, id: id, run: run}) + q.metrics.queueDepth.Record(context.Background(), int64(len(q.queue))) + q.lock.Unlock() + q.cond.Signal() + return true +} + func (q *RootScheduler) SubmitPeriodicJob(queue, id string, interval time.Duration, run func(ctx context.Context) error) { if q.ctx.Err() != nil || q.isDraining() { return diff --git a/internal/s3client/s3clienttest/s3clienttest.go b/internal/s3client/s3clienttest/s3clienttest.go index 75f8f0a8..b0a5d8bd 100644 --- a/internal/s3client/s3clienttest/s3clienttest.go +++ b/internal/s3client/s3clienttest/s3clienttest.go @@ -134,7 +134,7 @@ func startContainer(t *testing.T) { "-p", Port+":9000", "-e", "MINIO_ROOT_USER="+Username, "-e", "MINIO_ROOT_PASSWORD="+Password, - "minio/minio", "server", "/data", + "quay.io/minio/minio", "server", "/data", ) output, err := cmd.CombinedOutput() if err == nil { diff --git a/internal/strategy/git/clone_ownership_test.go b/internal/strategy/git/clone_ownership_test.go new file mode 100644 index 00000000..cf59ae7e --- /dev/null +++ b/internal/strategy/git/clone_ownership_test.go @@ -0,0 +1,58 @@ +package git //nolint:testpackage // This test needs access to the spool mutex. + +import ( + "context" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/logging" +) + +func TestCloneFailurePreservesSuccessorOwnership(t *testing.T) { + root := t.TempDir() + started := filepath.Join(root, "started") + assert.NoError(t, os.WriteFile(filepath.Join(root, "git"), []byte("#!/bin/sh\ntouch \"$CLONE_STARTED\"\nexit 42\n"), 0o750)) + t.Setenv("PATH", root+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("CLONE_STARTED", started) + ctx := logging.ContextWithLogger(context.Background(), slog.Default()) + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: filepath.Join(root, "mirrors")}, nil) + assert.NoError(t, err) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, "https://example.test/example/repo") + assert.NoError(t, err) + s := &Strategy{cache: memCache, cloneManager: manager, metrics: newGitMetrics()} + + s.spoolsMu.Lock() + locked := true + t.Cleanup(func() { + if locked { + s.spoolsMu.Unlock() + } + }) + done := make(chan error, 1) + go func() { done <- s.startClone(ctx, repo) }() + deadline := time.Now().Add(5 * time.Second) + for { + _, err := os.Stat(started) + if err == nil && repo.State() == gitclone.StateEmpty { + break + } + if time.Now().After(deadline) { + t.Fatal("clone did not fail before spool cleanup") + } + time.Sleep(time.Millisecond) + } + assert.True(t, repo.TryStartCloning()) + s.spoolsMu.Unlock() + locked = false + assert.Error(t, <-done) + assert.Equal(t, gitclone.StateCloning, repo.State()) +} diff --git a/internal/strategy/git/cold_snapshot_test.go b/internal/strategy/git/cold_snapshot_test.go new file mode 100644 index 00000000..afd3c919 --- /dev/null +++ b/internal/strategy/git/cold_snapshot_test.go @@ -0,0 +1,553 @@ +package git_test + +import ( + "context" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/alecthomas/assert/v2" + "github.com/alecthomas/errors" + + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/githubapp" + "github.com/block/cachew/internal/jobscheduler" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/metadatadb" + "github.com/block/cachew/internal/strategy/git" +) + +const coldTestUpstream = "https://example.test/example/repo" + +type controlledGit struct { + countFile string + releaseFile string +} + +type authoritativeStatErrorCache struct { + cache.Cache +} + +func (c authoritativeStatErrorCache) AuthoritativeStat(context.Context, cache.Key, ...cache.Option) (http.Header, error) { + return nil, errors.New("authoritative stat unavailable") +} + +type coldPublicationGate struct { + cache.Cache + started chan struct{} + release chan struct{} + once sync.Once +} + +func (c *coldPublicationGate) Create(ctx context.Context, key cache.Key, headers http.Header, ttl time.Duration, options ...cache.Option) (cache.Writer, error) { + if key == cache.NewKey(coldTestUpstream+".snapshot") { + c.once.Do(func() { close(c.started) }) + select { + case <-c.release: + case <-ctx.Done(): + return nil, ctx.Err() + } + } + return c.Cache.Create(ctx, key, headers, ttl, options...) +} + +func installControlledGit(t *testing.T, failFirst bool) controlledGit { + t.Helper() + realGit, err := exec.LookPath("git") + assert.NoError(t, err) + root := t.TempDir() + upstream := filepath.Join(root, "upstream.git") + createTestMirrorRepo(t, upstream) + countFile := filepath.Join(root, "clone-count") + releaseFile := filepath.Join(root, "release") + binDir := filepath.Join(root, "bin") + assert.NoError(t, os.Mkdir(binDir, 0o750)) + script := `#!/bin/sh +set -eu +saw_clone=0 +saw_mirror=0 +last="" +for arg in "$@"; do + if [ "$arg" = "clone" ]; then saw_clone=1; fi + if [ "$arg" = "--mirror" ]; then saw_mirror=1; fi + last="$arg" +done +if [ "$saw_clone" = "1" ] && [ "$saw_mirror" = "1" ]; then + printf 'clone\n' >> "$CLONE_COUNT_FILE" + if [ "${FAIL_FIRST_CLONE:-0}" = "1" ] && [ "$(wc -l < "$CLONE_COUNT_FILE")" -eq 1 ]; then + exit 42 + fi + while [ ! -e "$CLONE_RELEASE_FILE" ]; do sleep 0.01; done + exec "$REAL_GIT" clone --mirror "$FAKE_UPSTREAM" "$last" +fi +exec "$REAL_GIT" "$@" +` + assert.NoError(t, os.WriteFile(filepath.Join(binDir, "git"), []byte(script), 0o750)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("REAL_GIT", realGit) + t.Setenv("FAKE_UPSTREAM", upstream) + t.Setenv("CLONE_COUNT_FILE", countFile) + t.Setenv("CLONE_RELEASE_FILE", releaseFile) + if failFirst { + t.Setenv("FAIL_FIRST_CLONE", "1") + } + resolvedGit, err := exec.LookPath("git") + assert.NoError(t, err) + assert.Equal(t, filepath.Join(binDir, "git"), resolvedGit) + return controlledGit{countFile: countFile, releaseFile: releaseFile} +} + +func (c controlledGit) release(t *testing.T) { + t.Helper() + assert.NoError(t, os.WriteFile(c.releaseFile, nil, 0o600)) +} + +func (c controlledGit) count() int { + body, err := os.ReadFile(c.countFile) + if err != nil { + return 0 + } + return strings.Count(string(body), "clone\n") +} + +func waitForColdCondition(t *testing.T, condition func() bool) { + t.Helper() + deadline := time.Now().Add(30 * time.Second) + for time.Now().Before(deadline) { + if condition() { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("condition not met") +} + +func newColdSnapshotStrategy( + ctx context.Context, + t *testing.T, + scheduler jobscheduler.Provider, + c cache.Cache, + mirrorRoot string, +) (*git.Strategy, *testMux, *gitclone.Manager) { + return newColdSnapshotStrategyWithConfig(ctx, t, scheduler, c, mirrorRoot, git.Config{}) +} + +func newColdSnapshotStrategyWithConfig( + ctx context.Context, + t *testing.T, + scheduler jobscheduler.Provider, + c cache.Cache, + mirrorRoot string, + config git.Config, +) (*git.Strategy, *testMux, *gitclone.Manager) { + t.Helper() + mux := newTestMux() + managerProvider := gitclone.NewManagerProvider(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) + strategy, err := git.New(ctx, config, scheduler, c, mux, managerProvider, + func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil + assert.NoError(t, err) + if config.SnapshotInterval > 0 { + strategy.SetMetadataStore(nil) + } + waitForReady(t, strategy) + manager, err := managerProvider() + assert.NoError(t, err) + return strategy, mux, manager +} + +func coldSnapshotRequest(ctx context.Context, mux *testMux) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, "/git/example.test/example/repo/snapshot.tar.zst", nil).WithContext(ctx) + req.SetPathValue("host", "example.test") + req.SetPathValue("path", "example/repo/snapshot.tar.zst") + w := httptest.NewRecorder() + mux.handlers["GET /git/{host}/{path...}"].ServeHTTP(w, req) + return w +} + +func TestColdSnapshotMissIsFastCoalescedAndSurvivesDisconnect(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategy(ctx, t, newTestScheduler(ctx, t), memCache, + filepath.Join(t.TempDir(), "mirrors")) + + requestCtx, cancelRequest := context.WithCancel(ctx) + response := make(chan *httptest.ResponseRecorder, 1) + go func() { response <- coldSnapshotRequest(requestCtx, mux) }() + select { + case w := <-response: + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Equal(t, "no-store", w.Header().Get("Cache-Control")) + case <-time.After(time.Second): + controlled.release(t) + t.Fatal("cold snapshot miss blocked on mirror clone") + } + cancelRequest() + waitForColdCondition(t, func() bool { return controlled.count() >= 1 }) + assert.Equal(t, 1, controlled.count()) + + const requests = 16 + responses := make(chan int, requests) + var wg sync.WaitGroup + for range requests { + wg.Go(func() { responses <- coldSnapshotRequest(ctx, mux).Code }) + } + wg.Wait() + close(responses) + for status := range responses { + assert.Equal(t, http.StatusNotFound, status) + } + assert.Equal(t, 1, controlled.count()) + + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + controlled.release(t) + waitForColdCondition(t, func() bool { return repo.State() == gitclone.StateReady }) + assert.Equal(t, 1, controlled.count()) + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) + _, err = cache.StatAuthoritative(ctx, memCache, cache.NewKey(coldTestUpstream+".snapshot")) + assert.True(t, errors.Is(err, os.ErrNotExist)) +} + +func TestColdSnapshotPreparationFailureCanRetry(t *testing.T) { + controlled := installControlledGit(t, true) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategy(ctx, t, newTestScheduler(ctx, t), memCache, + filepath.Join(t.TempDir(), "mirrors")) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + waitForColdCondition(t, func() bool { + return controlled.count() >= 1 && !strategy.MirrorPreparationScheduled(coldTestUpstream) + }) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + assert.Equal(t, gitclone.StateEmpty, repo.State()) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + waitForColdCondition(t, func() bool { return controlled.count() >= 2 }) + controlled.release(t) + waitForColdCondition(t, func() bool { return repo.State() == gitclone.StateReady }) + assert.Equal(t, 2, controlled.count()) +} + +func TestColdSnapshotPreparationRetriesAfterCompetingCloneFails(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategy(ctx, t, newTestScheduler(ctx, t), memCache, + filepath.Join(t.TempDir(), "mirrors")) + repo, err := manager.GetOrCreate(ctx, coldTestUpstream) + assert.NoError(t, err) + assert.True(t, repo.TryStartCloning()) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + waitForColdCondition(t, func() bool { return strategy.MirrorPreparationScheduled(coldTestUpstream) }) + time.Sleep(600 * time.Millisecond) + assert.Equal(t, 0, controlled.count()) + repo.ResetToEmpty() + waitForColdCondition(t, func() bool { return controlled.count() >= 1 }) + controlled.release(t) + waitForColdCondition(t, func() bool { return repo.State() == gitclone.StateReady }) + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) +} + +func TestCoordinatedSnapshotFailureReleasesClaim(t *testing.T) { + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, _, manager := newColdSnapshotStrategy(ctx, t, newTestScheduler(ctx, t), memCache, + filepath.Join(t.TempDir(), "mirrors")) + store := metadatadb.New(ctx, metadatadb.NewMemoryBackend()) + prime := metadatadb.NewMap[string, string](store.Namespace("git"), "prime") + assert.NoError(t, prime.Set("key", "value")) + strategy.SetMetadataStore(store) + repo, err := manager.GetOrCreate(ctx, coldTestUpstream) + assert.NoError(t, err) + assert.Error(t, strategy.RunFailingCoordinatedSnapshot(ctx, repo)) + + claimed, err := strategy.ClaimSnapshotForTest(coldTestUpstream) + assert.NoError(t, err) + assert.True(t, claimed) +} + +func TestColdSnapshotPreparationContinuesAfterSharedStatError(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategy(ctx, t, newTestScheduler(ctx, t), + authoritativeStatErrorCache{Cache: memCache}, filepath.Join(t.TempDir(), "mirrors")) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + waitForColdCondition(t, func() bool { return controlled.count() >= 1 }) + controlled.release(t) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + waitForColdCondition(t, func() bool { return repo.State() == gitclone.StateReady }) + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) +} + +func TestColdSnapshotPreparationReusesSharedPublication(t *testing.T) { + mirrorRoot := filepath.Join(t.TempDir(), "mirrors") + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + schedulerCtx, cancelScheduler := context.WithCancel(ctx) + scheduler, err := jobscheduler.New(schedulerCtx, jobscheduler.Config{Concurrency: 1}) + assert.NoError(t, err) + t.Cleanup(func() { + cancelScheduler() + scheduler.Wait() + assert.NoError(t, scheduler.Close()) + }) + + blockerStarted := make(chan struct{}) + releaseBlocker := make(chan struct{}) + scheduler.Submit("blocker", "blocker", func(context.Context) error { + close(blockerStarted) + <-releaseBlocker + return nil + }) + <-blockerStarted + + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + provider := func() (*jobscheduler.RootScheduler, error) { return scheduler, nil } + strategy, mux, manager := newColdSnapshotStrategyWithConfig(ctx, t, provider, memCache, + mirrorRoot, git.Config{SnapshotInterval: time.Hour}) + strategy.SetColdPreparationDelay(func() time.Duration { return 0 }) + strategy.SetMetadataStore(metadatadb.New(ctx, metadatadb.NewMemoryBackend())) + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + assert.True(t, strategy.MirrorPreparationScheduled(coldTestUpstream)) + + err = cache.WriteFunc(ctx, memCache, cache.NewKey(coldTestUpstream+".snapshot"), nil, time.Hour, + func(w io.Writer) error { + _, err := w.Write([]byte("shared snapshot")) + return err + }) + assert.NoError(t, err) + close(releaseBlocker) + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + assert.Equal(t, gitclone.StateEmpty, repo.State()) +} + +func TestColdSnapshotPreparationRejectsWhenSchedulerQueueIsFull(t *testing.T) { + mirrorRoot := filepath.Join(t.TempDir(), "mirrors") + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + schedulerCtx, cancelScheduler := context.WithCancel(ctx) + scheduler, err := jobscheduler.New(schedulerCtx, jobscheduler.Config{Concurrency: 1}) + assert.NoError(t, err) + t.Cleanup(func() { + cancelScheduler() + scheduler.Wait() + assert.NoError(t, scheduler.Close()) + }) + + blockerStarted := make(chan struct{}) + releaseBlocker := make(chan struct{}) + scheduler.Submit("blocker", "blocker", func(context.Context) error { + close(blockerStarted) + <-releaseBlocker + return nil + }) + <-blockerStarted + t.Cleanup(func() { close(releaseBlocker) }) + for i := range git.ColdPreparationQueueLimitForTest() { + scheduler.Submit(fmt.Sprintf("queued-%d", i), "queued", func(context.Context) error { return nil }) + } + + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + provider := func() (*jobscheduler.RootScheduler, error) { return scheduler, nil } + strategy, mux, manager := newColdSnapshotStrategy(ctx, t, provider, memCache, mirrorRoot) + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + assert.False(t, strategy.MirrorPreparationScheduled(coldTestUpstream)) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + assert.Equal(t, gitclone.StateEmpty, repo.State()) +} + +func TestColdSnapshotPreparationRechecksPublicationAfterCoordinationDelay(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategyWithConfig(ctx, t, newTestScheduler(ctx, t), memCache, + filepath.Join(t.TempDir(), "mirrors"), git.Config{SnapshotInterval: time.Hour}) + strategy.SetMetadataStore(metadatadb.New(ctx, metadatadb.NewMemoryBackend())) + delayStarted := make(chan struct{}) + strategy.SetColdPreparationDelay(func() time.Duration { + close(delayStarted) + return 200 * time.Millisecond + }) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + <-delayStarted + err = cache.WriteFunc(ctx, memCache, cache.NewKey(coldTestUpstream+".snapshot"), nil, time.Hour, + func(w io.Writer) error { + _, err := w.Write([]byte("shared snapshot")) + return err + }) + assert.NoError(t, err) + + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) + assert.Equal(t, 0, controlled.count()) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + assert.Equal(t, gitclone.StateEmpty, repo.State()) +} + +func TestColdSnapshotPreparationPublishesFromReadyMirror(t *testing.T) { + mirrorRoot := filepath.Join(t.TempDir(), "mirrors") + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + schedulerCtx, cancelScheduler := context.WithCancel(ctx) + scheduler, err := jobscheduler.New(schedulerCtx, jobscheduler.Config{Concurrency: 1}) + assert.NoError(t, err) + t.Cleanup(func() { + cancelScheduler() + scheduler.Wait() + assert.NoError(t, scheduler.Close()) + }) + + blockerStarted := make(chan struct{}) + releaseBlocker := make(chan struct{}) + scheduler.Submit("blocker", "blocker", func(context.Context) error { + close(blockerStarted) + <-releaseBlocker + return nil + }) + <-blockerStarted + + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + gate := &coldPublicationGate{Cache: memCache, started: make(chan struct{}), release: make(chan struct{})} + unblock := sync.OnceFunc(func() { close(gate.release) }) + t.Cleanup(unblock) + provider := func() (*jobscheduler.RootScheduler, error) { return scheduler, nil } + strategy, mux, manager := newColdSnapshotStrategyWithConfig(ctx, t, provider, gate, + mirrorRoot, git.Config{SnapshotInterval: time.Hour, RepackInterval: time.Hour}) + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + createTestMirrorRepo(t, repo.Path()) + repo.MarkReady() + close(releaseBlocker) + + select { + case <-gate.started: + case <-time.After(30 * time.Second): + t.Fatal("cold snapshot did not reach publication") + } + assert.True(t, strategy.MirrorPreparationScheduled(coldTestUpstream)) + response := coldSnapshotRequest(ctx, mux) + assert.Equal(t, http.StatusNotFound, response.Code) + assert.Equal(t, "no-store", response.Header().Get("Cache-Control")) + unblock() + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) + snapshotScheduled, repackScheduled := strategy.PeriodicJobsScheduled(coldTestUpstream) + assert.True(t, snapshotScheduled) + assert.True(t, repackScheduled) + _, err = cache.StatAuthoritative(ctx, memCache, cache.NewKey(coldTestUpstream+".snapshot")) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, coldSnapshotRequest(ctx, mux).Code) +} + +func TestColdSnapshotPreparationCoordinatesAcrossReplicas(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + sharedCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + store := metadatadb.New(ctx, metadatadb.NewMemoryBackend()) + prime := metadatadb.NewMap[string, string](store.Namespace("git"), "prime") + assert.NoError(t, prime.Set("key", "value")) + + newReplica := func(mirrorRoot string) (*git.Strategy, *testMux, *gitclone.Manager) { + mux := newTestMux() + managerProvider := gitclone.NewManagerProvider(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) + strategy, err := git.New(ctx, git.Config{SnapshotInterval: time.Hour}, newTestScheduler(ctx, t), sharedCache, mux, + managerProvider, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil + assert.NoError(t, err) + strategy.SetColdPreparationDelay(func() time.Duration { return 0 }) + strategy.SetMetadataStore(store) + waitForColdCondition(t, strategy.Ready) + manager, err := managerProvider() + assert.NoError(t, err) + return strategy, mux, manager + } + + strategyA, muxA, managerA := newReplica(filepath.Join(t.TempDir(), "mirror-a")) + strategyB, muxB, managerB := newReplica(filepath.Join(t.TempDir(), "mirror-b")) + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, muxA).Code) + waitForColdCondition(t, func() bool { return controlled.count() >= 1 }) + assert.Equal(t, 1, controlled.count()) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, muxB).Code) + waitForColdCondition(t, func() bool { return !strategyB.MirrorPreparationScheduled(coldTestUpstream) }) + assert.Equal(t, 1, controlled.count()) + repoB := managerB.Get(coldTestUpstream) + assert.True(t, repoB != nil) + assert.Equal(t, gitclone.StateEmpty, repoB.State()) + + controlled.release(t) + repoA := managerA.Get(coldTestUpstream) + assert.True(t, repoA != nil) + waitForColdCondition(t, func() bool { return repoA.State() == gitclone.StateReady }) + waitForColdCondition(t, func() bool { return !strategyA.MirrorPreparationScheduled(coldTestUpstream) }) + assert.Equal(t, 1, controlled.count()) + assert.Equal(t, http.StatusOK, coldSnapshotRequest(ctx, muxB).Code) + assert.Equal(t, 1, controlled.count()) + + assert.NoError(t, sharedCache.Delete(ctx, cache.NewKey(coldTestUpstream+".snapshot"))) + strategyC, muxC, managerC := newReplica(filepath.Join(t.TempDir(), "mirror-c")) + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, muxC).Code) + waitForColdCondition(t, func() bool { return controlled.count() >= 2 }) + repoC := managerC.Get(coldTestUpstream) + assert.True(t, repoC != nil) + waitForColdCondition(t, func() bool { return repoC.State() == gitclone.StateReady }) + waitForColdCondition(t, func() bool { return !strategyC.MirrorPreparationScheduled(coldTestUpstream) }) +} + +func TestColdSnapshotPreparationStopsOnSchedulerShutdown(t *testing.T) { + controlled := installControlledGit(t, false) + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + schedulerCtx, cancelScheduler := context.WithCancel(ctx) + scheduler, err := jobscheduler.New(schedulerCtx, jobscheduler.Config{Concurrency: 1}) + assert.NoError(t, err) + provider := func() (*jobscheduler.RootScheduler, error) { return scheduler, nil } + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + strategy, mux, manager := newColdSnapshotStrategyWithConfig(ctx, t, provider, memCache, + filepath.Join(t.TempDir(), "mirrors"), git.Config{SnapshotInterval: time.Hour}) + strategy.SetColdPreparationDelay(func() time.Duration { return 0 }) + strategy.SetMetadataStore(metadatadb.New(ctx, metadatadb.NewMemoryBackend())) + + assert.Equal(t, http.StatusNotFound, coldSnapshotRequest(ctx, mux).Code) + waitForColdCondition(t, func() bool { return controlled.count() >= 1 }) + cancelScheduler() + scheduler.Wait() + assert.NoError(t, scheduler.Close()) + waitForColdCondition(t, func() bool { return !strategy.MirrorPreparationScheduled(coldTestUpstream) }) + repo := manager.Get(coldTestUpstream) + assert.True(t, repo != nil) + assert.Equal(t, gitclone.StateEmpty, repo.State()) + claimed, err := strategy.ClaimSnapshotForTest(coldTestUpstream) + assert.NoError(t, err) + assert.True(t, claimed) + leftovers, err := filepath.Glob(filepath.Join(filepath.Dir(repo.Path()), ".clone-*")) + assert.NoError(t, err) + assert.Equal(t, 0, len(leftovers)) +} diff --git a/internal/strategy/git/export_test.go b/internal/strategy/git/export_test.go index 4ccbff3e..528bee2e 100644 --- a/internal/strategy/git/export_test.go +++ b/internal/strategy/git/export_test.go @@ -5,6 +5,8 @@ import ( "io" "time" + "github.com/alecthomas/errors" + "github.com/block/cachew/internal/cache" "github.com/block/cachew/internal/gitclone" ) @@ -30,7 +32,42 @@ func (s *Strategy) RunCoordinatedSnapshot(ctx context.Context, repo *gitclone.Re })(ctx) } +// RunFailingCoordinatedSnapshot causes a snapshot job to fail so tests can check claim release. +func (s *Strategy) RunFailingCoordinatedSnapshot(ctx context.Context, repo *gitclone.Repository) error { + return s.coordinatedSnapshotJob(snapshotJobBase, repo, 0, func(context.Context) (string, error) { + return "", errors.New("snapshot generation failed") + })(ctx) +} + // CacheBundle exports cacheBundle for testing. func (s *Strategy) CacheBundle(ctx context.Context, key cache.Key, r io.Reader) error { return s.cacheBundle(ctx, key, r) } + +// MirrorPreparationScheduled lets tests check for a queued or active preparation job. +func (s *Strategy) MirrorPreparationScheduled(upstream string) bool { + _, ok := s.mirrorPreparations.Load(upstream) + return ok +} + +// PeriodicJobsScheduled lets tests check the periodic jobs that the strategy registered. +func (s *Strategy) PeriodicJobsScheduled(upstream string) (snapshot, repack bool) { + _, snapshot = s.snapshotJobsScheduled.Load(upstream) + _, repack = s.repackJobsScheduled.Load(upstream) + return snapshot, repack +} + +// SetColdPreparationDelay lets tests control the delay before a snapshot claim. +func (s *Strategy) SetColdPreparationDelay(delay func() time.Duration) { + s.coldPreparationDelay = delay +} + +// ColdPreparationQueueLimitForTest returns the queue limit for tests in external packages. +func ColdPreparationQueueLimitForTest() int { + return coldPreparationQueueLimit +} + +// ClaimSnapshotForTest lets tests request a base snapshot claim. +func (s *Strategy) ClaimSnapshotForTest(upstream string) (bool, error) { + return s.snapshotCoord.Claim(snapshotJobBase, upstream, 0) +} diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index 923fc18e..25957fe6 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "io" "maps" + "math/rand/v2" "net/http" "net/http/httputil" "os" @@ -55,26 +56,30 @@ type Config struct { } type Strategy struct { - config Config - cache cache.Cache - cloneManager *gitclone.Manager - httpClient *http.Client - proxy *httputil.ReverseProxy - ctx context.Context - scheduler jobscheduler.Scheduler - spoolsMu sync.Mutex - spools map[string]*RepoSpools - tokenManager *githubapp.TokenManager - snapshotMu sync.Map // keyed by upstream URL, values are *sync.Mutex - snapshotSpools sync.Map // keyed by upstream URL, values are *snapshotSpoolEntry - coldSnapshotMu sync.Map // keyed by upstream URL, values are *coldSnapshotEntry - deferredRestoreOnce sync.Map // keyed by upstream URL, ensures at most one deferred restore per repo - metrics *gitMetrics - repoCounts *RepoCounts - snapshotCoord *SnapshotCoordinator - metadataWired chan struct{} // closed by SetMetadataStore; gates warm-up - wiredOnce sync.Once - ready atomic.Bool + config Config + cache cache.Cache + cloneManager *gitclone.Manager + httpClient *http.Client + proxy *httputil.ReverseProxy + ctx context.Context + scheduler jobscheduler.Scheduler + spoolsMu sync.Mutex + spools map[string]*RepoSpools + tokenManager *githubapp.TokenManager + snapshotMu sync.Map // keyed by upstream URL, values are *sync.Mutex + snapshotSpools sync.Map // keyed by upstream URL, values are *snapshotSpoolEntry + snapshotJobsScheduled sync.Map // One entry per upstream URL prevents duplicate periodic jobs. + repackJobsScheduled sync.Map // One entry per upstream URL prevents duplicate periodic jobs. + coldSnapshotMu sync.Map // keyed by upstream URL, values are *coldSnapshotEntry + mirrorPreparations sync.Map // One entry per upstream URL prevents duplicate preparation jobs. + deferredRestoreOnce sync.Map // keyed by upstream URL, ensures at most one deferred restore per repo + metrics *gitMetrics + repoCounts *RepoCounts + snapshotCoord *SnapshotCoordinator + coldPreparationDelay func() time.Duration + metadataWired chan struct{} // closed by SetMetadataStore; gates warm-up + wiredOnce sync.Once + ready atomic.Bool } func New( @@ -145,6 +150,9 @@ func New( tokenManager: tokenManager, metrics: m, metadataWired: make(chan struct{}), + coldPreparationDelay: func() time.Duration { + return rand.N(coldPreparationSpread) //nolint:gosec // The delay does not protect sensitive data. + }, } // Run startup fetches in the background so the HTTP listener (and // /_liveness) come up immediately. /_readiness gates on Ready() so the @@ -575,24 +583,24 @@ func ExtractRepoPath(pathValue string) string { // goroutine is already cloning (StateCloning), it polls until completion or the // context is cancelled. Returns an error if the clone fails or the context is done. func (s *Strategy) ensureCloneReady(ctx context.Context, repo *gitclone.Repository) error { - if repo.State() == gitclone.StateEmpty { - if err := s.startClone(ctx, repo); err != nil { - return err - } - } - for repo.State() == gitclone.StateCloning { - t := time.NewTimer(500 * time.Millisecond) - select { - case <-ctx.Done(): - t.Stop() - return errors.Wrap(ctx.Err(), "cancelled waiting for clone") - case <-t.C: + for { + switch repo.State() { + case gitclone.StateReady: + return nil + case gitclone.StateEmpty: + if err := s.startClone(ctx, repo); err != nil { + return err + } + case gitclone.StateCloning: + t := time.NewTimer(500 * time.Millisecond) + select { + case <-ctx.Done(): + t.Stop() + return errors.Wrap(ctx.Err(), "cancelled waiting for clone") + case <-t.C: + } } } - if repo.State() != gitclone.StateReady { - return errors.New("repository unavailable after clone attempt") - } - return nil } func (s *Strategy) startClone(ctx context.Context, repo *gitclone.Repository) (returnErr error) { @@ -623,8 +631,11 @@ func (s *Strategy) startClone(ctx context.Context, repo *gitclone.Repository) (r logger.InfoContext(ctx, "Attempting mirror snapshot restore", "upstream", upstream) - if err := s.tryRestoreSnapshot(ctx, repo); err != nil { - logger.InfoContext(ctx, "Mirror snapshot restore failed, falling back to clone", "upstream", upstream, "error", err) + restoreCtx, cancelRestore := context.WithTimeout(ctx, s.cloneManager.Config().CloneTimeout) + restoreErr := s.tryRestoreSnapshot(restoreCtx, repo) + cancelRestore() + if restoreErr != nil { + logger.InfoContext(ctx, "Mirror snapshot restore failed, falling back to clone", "upstream", upstream, "error", restoreErr) } else { logger.InfoContext(ctx, "Mirror snapshot restored, fetching to freshen", "upstream", upstream) @@ -641,8 +652,8 @@ func (s *Strategy) startClone(ctx context.Context, repo *gitclone.Repository) (r "upstream", upstream, "error", err) // The restored snapshot may be corrupt or empty. Remove it and // fall through to a fresh clone so we don't re-upload bad data. - repo.ResetToEmpty() if rmErr := os.RemoveAll(repo.Path()); rmErr != nil { + repo.ResetToEmpty() return errors.Wrapf(rmErr, "remove corrupt mirror for %s", upstream) } } else { @@ -667,7 +678,7 @@ func (s *Strategy) startClone(ctx context.Context, repo *gitclone.Repository) (r logger.InfoContext(ctx, "Starting clone", "upstream", upstream, "path", repo.Path()) cloneStart := time.Now() - err := repo.Clone(ctx) + err := repo.CloneClaimed(ctx) // Clean up spools regardless of clone success or failure, so that subsequent // requests either serve from the local backend or go directly to upstream. @@ -677,7 +688,6 @@ func (s *Strategy) startClone(ctx context.Context, repo *gitclone.Repository) (r if err != nil { s.metrics.recordOperation(ctx, "clone", "error", time.Since(cloneStart)) - repo.ResetToEmpty() return errors.Wrapf(err, "clone %s", upstream) } diff --git a/internal/strategy/git/repack.go b/internal/strategy/git/repack.go index 8a428532..3fc1f1c7 100644 --- a/internal/strategy/git/repack.go +++ b/internal/strategy/git/repack.go @@ -16,6 +16,9 @@ import ( ) func (s *Strategy) scheduleRepackJobs(repo *gitclone.Repository) { + if _, loaded := s.repackJobsScheduled.LoadOrStore(repo.UpstreamURL(), true); loaded { + return + } s.scheduler.SubmitPeriodicJob(repo.UpstreamURL(), "repack-periodic", s.config.RepackInterval, func(ctx context.Context) (returnErr error) { upstream := repo.UpstreamURL() ctx, span := tracer.Start(ctx, "git.repack", diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index b514f8df..e5f071b5 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -279,6 +279,12 @@ const ( snapshotJobMirror = "mirror-snapshot" ) +const coldPreparationSpread = 30 * time.Second + +const coldPreparationQueueLimit = 128 + +const snapshotClaimCleanupTimeout = 30 * time.Second + // snapshotUnchanged reports whether generation can be skipped: the last // completed generation captured the same commit recently enough that its // cache entry cannot have expired, and the entry still exists in @@ -301,10 +307,23 @@ func (s *Strategy) snapshotUnchanged(ctx context.Context, job string, key cache. const snapshotStartupSpread = 5 * time.Minute func (s *Strategy) scheduleSnapshotJobs(repo *gitclone.Repository) { + if _, preparing := s.mirrorPreparations.Load(repo.UpstreamURL()); preparing { + return + } + s.scheduleSnapshotJobsAfter(repo, 0) +} + +func (s *Strategy) scheduleSnapshotJobsAfter(repo *gitclone.Repository, baseDelay time.Duration) { upstream := repo.UpstreamURL() + if _, loaded := s.snapshotJobsScheduled.LoadOrStore(upstream, true); loaded { + return + } submit := func(job string, interval time.Duration, generate func(ctx context.Context) (string, error)) { run := s.coordinatedSnapshotJob(job, repo, interval, generate) delay, interval := s.snapshotSchedule(interval) + if job == snapshotJobBase { + delay = max(delay, baseDelay) + } if delay == 0 { s.scheduler.SubmitPeriodicJob(upstream, job+"-periodic", interval, run) return @@ -358,34 +377,52 @@ func (s *Strategy) coordinatedSnapshotJob(job string, repo *gitclone.Repository, upstream := repo.UpstreamURL() return func(ctx context.Context) error { logger := logging.FromContext(ctx) - claimed, err := s.snapshotCoord.Claim(job, upstream, interval) + claimID, claimed, err := s.snapshotCoord.ClaimWithTTL(job, upstream, interval, snapshotClaimTTL) if err != nil { logger.WarnContext(ctx, "Snapshot coordination claim failed, generating anyway", "job", job, "upstream", upstream, "error", err) + claimID = "" } else if !claimed { logger.DebugContext(ctx, "Skipping snapshot generation, fresh or in progress on another replica", "job", job, "upstream", upstream) return nil } commit, err := generate(ctx) if err != nil { - return err + return errors.Join(err, s.failSnapshotClaim(ctx, job, upstream, claimID)) } if commit == "" { // Nothing was uploaded, so record a skip rather than a // completion: CompletedAt must keep tracking the last actual // upload, while the skip's CheckedAt keeps peers from re-fetching // an unchanged repo every interval. - if err := s.snapshotCoord.Skip(job, upstream); err != nil { + if err := s.snapshotCoord.SkipClaim(ctx, job, upstream, claimID); err != nil { logger.WarnContext(ctx, "Failed to record snapshot skip", "job", job, "upstream", upstream, "error", err) + if releaseErr := s.failSnapshotClaim(ctx, job, upstream, claimID); releaseErr != nil { + logger.WarnContext(ctx, "Failed to release snapshot claim after skip error", "job", job, + "upstream", upstream, "error", releaseErr) + } } return nil } - if err := s.snapshotCoord.Complete(job, upstream, commit); err != nil { + if err := s.snapshotCoord.CompleteClaim(ctx, job, upstream, claimID, commit); err != nil { logger.WarnContext(ctx, "Failed to record snapshot completion", "job", job, "upstream", upstream, "error", err) + if releaseErr := s.failSnapshotClaim(ctx, job, upstream, claimID); releaseErr != nil { + logger.WarnContext(ctx, "Failed to release snapshot claim after completion error", "job", job, + "upstream", upstream, "error", releaseErr) + } } return nil } } +func (s *Strategy) failSnapshotClaim(ctx context.Context, job, upstream, claimID string) error { + if claimID == "" { + return nil + } + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), snapshotClaimCleanupTimeout) + defer cancel() + return s.snapshotCoord.Fail(cleanupCtx, job, upstream, claimID) +} + // jitterInterval spreads replicas' periodic snapshot schedules apart so that // coordination claims (synced asynchronously between replicas) propagate // before a peer decides whether to generate. Without it, replicas deployed @@ -479,11 +516,10 @@ func (s *Strategy) handleSnapshotRequest(w http.ResponseWriter, r *http.Request, } } - // Either the mirror is already ready or no cached snapshot exists — fall - // through to the original path which blocks until the mirror is available. - if cloneErr := s.ensureCloneReady(ctx, repo); cloneErr != nil { - logger.ErrorContext(ctx, "Clone unavailable for snapshot", "upstream", upstreamURL, "error", cloneErr) - http.Error(w, "Repository unavailable", http.StatusServiceUnavailable) + if repo.State() != gitclone.StateReady { + s.scheduleColdSnapshotPreparation(ctx, repo) + w.Header().Set("Cache-Control", "no-store") + http.Error(w, "Snapshot not cached", http.StatusNotFound) return } @@ -503,6 +539,11 @@ func (s *Strategy) handleSnapshotRequest(w http.ResponseWriter, r *http.Request, span.SetStatus(codes.Error, serveErr.Error()) } case errors.Is(err, os.ErrNotExist): + if _, preparing := s.mirrorPreparations.Load(upstreamURL); preparing { + w.Header().Set("Cache-Control", "no-store") + http.Error(w, "Snapshot not cached", http.StatusNotFound) + return + } if spoolErr := s.serveSnapshotWithSpool(w, r, repo, upstreamURL, repoName, start); spoolErr != nil { logger.ErrorContext(ctx, "Failed to serve snapshot via spool", "upstream", upstreamURL, "error", spoolErr) span.RecordError(spoolErr) @@ -523,6 +564,7 @@ func (s *Strategy) serveSnapshotHead(ctx context.Context, w http.ResponseWriter, headers, err := s.cache.Stat(ctx, cacheKey) if err != nil { if errors.Is(err, os.ErrNotExist) { + w.Header().Set("Cache-Control", "no-store") http.Error(w, "Snapshot not cached", http.StatusNotFound) return } @@ -1168,6 +1210,134 @@ func (s *Strategy) scheduleDeferredMirrorRestore(ctx context.Context, repo *gitc }) } +func (s *Strategy) scheduleColdSnapshotPreparation(ctx context.Context, repo *gitclone.Repository) { + upstream := repo.UpstreamURL() + if _, loaded := s.mirrorPreparations.LoadOrStore(upstream, true); loaded { + return + } + + logging.FromContext(ctx).InfoContext(ctx, "Scheduling cold snapshot preparation", "upstream", upstream) + accepted := s.scheduler.TrySubmit(upstream, "cold-snapshot-clone", coldPreparationQueueLimit, func(ctx context.Context) error { + defer s.mirrorPreparations.Delete(upstream) + ctx, cancel := context.WithTimeout(ctx, s.coldPreparationTimeout()) + defer cancel() + return s.prepareColdSnapshot(ctx, repo) + }) + if !accepted { + s.mirrorPreparations.Delete(upstream) + logging.FromContext(ctx).WarnContext(ctx, "Cold snapshot preparation queue is full", "upstream", upstream) + } +} + +func (s *Strategy) coldPreparationTimeout() time.Duration { + const maxDuration = time.Duration(1<<63 - 1) + cloneTimeout := s.cloneManager.Config().CloneTimeout + if cloneTimeout > (maxDuration-snapshotClaimTTL)/3 { + return maxDuration + } + return 3*cloneTimeout + snapshotClaimTTL +} + +func (s *Strategy) coldSnapshotPublished(ctx context.Context, upstream string) (bool, error) { + _, err := cache.StatAuthoritative(ctx, s.cache, snapshotCacheKey(upstream)) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return err == nil, errors.Wrap(err, "check shared snapshot") +} + +func (s *Strategy) prepareColdSnapshot(ctx context.Context, repo *gitclone.Repository) (returnErr error) { + upstream := repo.UpstreamURL() + defer func() { + if repo.State() == gitclone.StateReady && s.config.SnapshotInterval > 0 { + s.scheduleSnapshotJobsAfter(repo, s.config.SnapshotInterval) + } + }() + if published, err := s.coldSnapshotPublished(ctx, upstream); err != nil { + logging.FromContext(ctx).WarnContext(ctx, "Shared snapshot recheck failed, preparing anyway", + "upstream", upstream, "error", err) + } else if published { + return nil + } + if repo.State() == gitclone.StateReady && s.config.SnapshotInterval == 0 { + return nil + } + + claimID, claimed := "", true + if s.config.SnapshotInterval > 0 && s.snapshotCoord != nil { + timer := time.NewTimer(s.coldPreparationDelay()) + select { + case <-ctx.Done(): + timer.Stop() + return errors.Wrap(ctx.Err(), "wait to coordinate cold snapshot preparation") + case <-timer.C: + } + primeErr := s.snapshotCoord.Prime(ctx) + if primeErr != nil { + logging.FromContext(ctx).WarnContext(ctx, "Cold preparation coordination refresh failed, preparing anyway", + "upstream", upstream, "error", primeErr) + } + if published, err := s.coldSnapshotPublished(ctx, upstream); err != nil { + logging.FromContext(ctx).WarnContext(ctx, "Shared snapshot recheck failed, preparing anyway", + "upstream", upstream, "error", err) + } else if published { + return nil + } + if primeErr == nil { + var err error + claimID, claimed, err = s.snapshotCoord.ClaimWithTTL(snapshotJobBase, upstream, 0, + s.coldPreparationTimeout()) + if err != nil { + logging.FromContext(ctx).WarnContext(ctx, "Cold preparation coordination claim failed, preparing anyway", + "upstream", upstream, "error", err) + claimed = true + } + } + } + if !claimed { + return nil + } + if claimID != "" { + defer func() { + if returnErr != nil { + returnErr = errors.Join(returnErr, s.failSnapshotClaim(ctx, snapshotJobBase, upstream, claimID)) + } + }() + } + + if err := s.ensureCloneReady(ctx, repo); err != nil { + return err + } + if s.config.RepackInterval > 0 { + s.scheduleRepackJobs(repo) + } + publishedCommit := "" + if s.config.SnapshotInterval > 0 { + commit, err := s.generateAndUploadSnapshot(ctx, repo) + if err != nil { + return errors.Wrap(err, "publish prepared snapshot") + } + publishedCommit = commit + } + if claimID != "" { + var err error + if publishedCommit == "" { + err = s.snapshotCoord.SkipClaim(ctx, snapshotJobBase, upstream, claimID) + } else { + err = s.snapshotCoord.CompleteClaim(ctx, snapshotJobBase, upstream, claimID, publishedCommit) + } + if err != nil { + logging.FromContext(ctx).WarnContext(ctx, "Failed to record cold snapshot preparation result", + "upstream", upstream, "error", err) + if releaseErr := s.failSnapshotClaim(ctx, snapshotJobBase, upstream, claimID); releaseErr != nil { + logging.FromContext(ctx).WarnContext(ctx, "Failed to release cold snapshot claim after recording error", + "upstream", upstream, "error", releaseErr) + } + } + } + return nil +} + // snapshotSpoolEntry holds a spool and a ready channel used to coordinate // writer election. The first goroutine stores the entry via LoadOrStore and // becomes the writer. It closes ready once the spool is created (or on diff --git a/internal/strategy/git/snapshot_test.go b/internal/strategy/git/snapshot_test.go index 739f86aa..bece9888 100644 --- a/internal/strategy/git/snapshot_test.go +++ b/internal/strategy/git/snapshot_test.go @@ -84,8 +84,6 @@ func TestSnapshotHTTPEndpoint(t *testing.T) { assert.Equal(t, "application/zstd", w.Header().Get("Content-Type")) assert.Equal(t, snapshotData, w.Body.Bytes()) - // Test snapshot not found - repo has no mirror, so clone is attempted but - // fails immediately because the context is cancelled. cancelCtx, cancel := context.WithCancel(ctx) cancel() req = httptest.NewRequest(http.MethodGet, "/git/github.com/org/nonexistent/snapshot.tar.zst", nil) @@ -96,7 +94,8 @@ func TestSnapshotHTTPEndpoint(t *testing.T) { handler.ServeHTTP(w, req) - assert.Equal(t, 503, w.Code) + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Equal(t, "no-store", w.Header().Get("Cache-Control")) } func TestSnapshotOnDemandGenerationViaHTTP(t *testing.T) { @@ -758,6 +757,7 @@ func TestSnapshotHeadServesMetadataWithoutBody(t *testing.T) { missResp := httptest.NewRecorder() handler.ServeHTTP(missResp, missReq) assert.Equal(t, 404, missResp.Code, "HEAD on an uncached snapshot must not trigger generation") + assert.Equal(t, "no-store", missResp.Header().Get("Cache-Control")) waitForReady(t, s) err = s.GenerateAndUploadSnapshot(ctx, repo) diff --git a/internal/strategy/git/snapshotcoord.go b/internal/strategy/git/snapshotcoord.go index 9049df05..95854d2d 100644 --- a/internal/strategy/git/snapshotcoord.go +++ b/internal/strategy/git/snapshotcoord.go @@ -5,6 +5,7 @@ import ( "time" "github.com/alecthomas/errors" + "github.com/google/uuid" "github.com/block/cachew/internal/metadatadb" ) @@ -22,15 +23,16 @@ const ( snapshotClaimTTL = 30 * time.Minute ) -// snapshotGenRecord is the shared per-artifact generation state. StartedAt is -// the most recent claim; CompletedAt is the most recent successful generation; -// CheckedAt is the most recent claim that ended in a skip (nothing uploaded); -// Commit is the mirror HEAD that generation captured. +// snapshotGenRecord stores the generation state for each artifact. +// ClaimID identifies the owner for release and completion checks. +// ClaimExpiresAt gives all replicas the same expiration time. type snapshotGenRecord struct { - StartedAt time.Time `json:"started_at"` - CompletedAt time.Time `json:"completed_at,omitzero"` - CheckedAt time.Time `json:"checked_at,omitzero"` - Commit string `json:"commit,omitempty"` + StartedAt time.Time `json:"started_at"` + CompletedAt time.Time `json:"completed_at,omitzero"` + CheckedAt time.Time `json:"checked_at,omitzero"` + ClaimExpiresAt time.Time `json:"claim_expires_at,omitzero"` + Commit string `json:"commit,omitempty"` + ClaimID string `json:"claim_id,omitempty"` } // SnapshotCoordinator shares per-artifact generation state across replicas so @@ -76,8 +78,18 @@ func (c *SnapshotCoordinator) Prime(ctx context.Context) error { // completed a generation or checked-and-skipped within the interval, or // holds an unexpired in-progress claim. func (c *SnapshotCoordinator) Claim(job, upstreamURL string, interval time.Duration) (bool, error) { + _, claimed, err := c.ClaimWithTTL(job, upstreamURL, interval, snapshotClaimTTL) + return claimed, err +} + +// ClaimWithTTL lets the caller set the claim duration. +// The caller must use the returned claim ID to complete or release the claim. +func (c *SnapshotCoordinator) ClaimWithTTL(job, upstreamURL string, interval, claimTTL time.Duration) (string, bool, error) { if c == nil { - return true, nil + return "", true, nil + } + if claimTTL <= 0 { + claimTTL = snapshotClaimTTL } key := snapshotGenKey(job, upstreamURL) now := c.now() @@ -88,25 +100,31 @@ func (c *SnapshotCoordinator) Claim(job, upstreamURL string, interval time.Durat // just before the generator's next one sees an almost-interval-old // completion and skips rather than duplicating the imminent generation. if !rec.CompletedAt.IsZero() && now.Sub(rec.CompletedAt) < interval { - return false, nil + return "", false, nil } // A checked-and-skipped decision is as fresh as a completion for // claiming purposes: without it, once the last upload ages past the // interval every replica would re-fetch an unchanged repo each // interval instead of one. if !rec.CheckedAt.IsZero() && now.Sub(rec.CheckedAt) < interval { - return false, nil + return "", false, nil } inProgress := rec.CompletedAt.Before(rec.StartedAt) - if inProgress && now.Sub(rec.StartedAt) < snapshotClaimTTL { - return false, nil + claimExpiresAt := rec.ClaimExpiresAt + if claimExpiresAt.IsZero() { + claimExpiresAt = rec.StartedAt.Add(snapshotClaimTTL) + } + if inProgress && now.Before(claimExpiresAt) { + return "", false, nil } } rec.StartedAt = now + rec.ClaimID = uuid.NewString() + rec.ClaimExpiresAt = now.Add(claimTTL) if err := c.gens.Set(key, rec); err != nil { - return true, errors.Wrap(err, "record snapshot claim") + return rec.ClaimID, true, errors.Wrap(err, "record snapshot claim") } - return true, nil + return rec.ClaimID, true, nil } // Complete records a successful generation and the commit it captured so @@ -119,9 +137,31 @@ func (c *SnapshotCoordinator) Complete(job, upstreamURL, commit string) error { rec, _ := c.gens.Get(key) rec.CompletedAt = c.now() rec.Commit = commit + rec.ClaimID = "" + rec.ClaimExpiresAt = time.Time{} return errors.Wrap(c.gens.Set(key, rec), "record snapshot completion") } +// CompleteClaim reads the shared state again. It completes the claim only if the claim ID still matches. +func (c *SnapshotCoordinator) CompleteClaim(ctx context.Context, job, upstreamURL, claimID, commit string) error { + if c == nil || claimID == "" { + return nil + } + if err := c.Prime(ctx); err != nil { + return errors.Wrap(err, "refresh before completing snapshot claim") + } + key := snapshotGenKey(job, upstreamURL) + rec, ok := c.gens.Get(key) + if !ok || rec.ClaimID != claimID { + return nil + } + rec.CompletedAt = c.now() + rec.Commit = commit + rec.ClaimID = "" + rec.ClaimExpiresAt = time.Time{} + return errors.Wrap(c.gens.Set(key, rec), "record owned snapshot completion") +} + // Skip records a claim that ended without an upload (unchanged artifact or // nothing to snapshot). It clears the in-progress claim and stamps CheckedAt // so peers stay suppressed for the freshness interval rather than the claim @@ -137,9 +177,53 @@ func (c *SnapshotCoordinator) Skip(job, upstreamURL string) error { if rec.CompletedAt.Before(rec.StartedAt) { rec.StartedAt = rec.CompletedAt } + rec.ClaimID = "" + rec.ClaimExpiresAt = time.Time{} return errors.Wrap(c.gens.Set(key, rec), "record snapshot skip") } +// SkipClaim reads the shared state again. It records a skip only if the claim ID still matches. +func (c *SnapshotCoordinator) SkipClaim(ctx context.Context, job, upstreamURL, claimID string) error { + if c == nil || claimID == "" { + return nil + } + if err := c.Prime(ctx); err != nil { + return errors.Wrap(err, "refresh before skipping snapshot claim") + } + key := snapshotGenKey(job, upstreamURL) + rec, ok := c.gens.Get(key) + if !ok || rec.ClaimID != claimID { + return nil + } + rec.CheckedAt = c.now() + if rec.CompletedAt.Before(rec.StartedAt) { + rec.StartedAt = rec.CompletedAt + } + rec.ClaimID = "" + rec.ClaimExpiresAt = time.Time{} + return errors.Wrap(c.gens.Set(key, rec), "record owned snapshot skip") +} + +// Fail releases a failed claim only if the claim ID still matches the shared state. +// It does not record a successful check, so another caller can try again immediately. +func (c *SnapshotCoordinator) Fail(ctx context.Context, job, upstreamURL, claimID string) error { + if c == nil || claimID == "" { + return nil + } + if err := c.Prime(ctx); err != nil { + return errors.Wrap(err, "refresh before releasing snapshot claim") + } + key := snapshotGenKey(job, upstreamURL) + rec, ok := c.gens.Get(key) + if !ok || rec.ClaimID != claimID || !rec.CompletedAt.Before(rec.StartedAt) { + return nil + } + rec.StartedAt = rec.CompletedAt + rec.ClaimID = "" + rec.ClaimExpiresAt = time.Time{} + return errors.Wrap(c.gens.Set(key, rec), "release failed snapshot claim") +} + // Unchanged reports whether the last completed generation captured the same // commit and is recent enough (within maxAge) that its cache entry cannot // have expired. Callers skipping on this must not call Complete, so diff --git a/internal/strategy/git/snapshotcoord_test.go b/internal/strategy/git/snapshotcoord_test.go index df4e06b2..40241592 100644 --- a/internal/strategy/git/snapshotcoord_test.go +++ b/internal/strategy/git/snapshotcoord_test.go @@ -2,12 +2,23 @@ package git //nolint:testpackage // white-box testing required for clock injecti import ( "context" + "fmt" + "io" "log/slog" + "net/http" + "os" + "os/exec" + "strings" "testing" + "testing/synctest" "time" "github.com/alecthomas/assert/v2" + "github.com/alecthomas/errors" + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/jobscheduler" "github.com/block/cachew/internal/logging" "github.com/block/cachew/internal/metadatadb" ) @@ -25,6 +36,156 @@ func newTestSnapshotCoordinators(t *testing.T, now func() time.Time, replicas in return coords } +type coldSnapshotProbeCache struct { + cache.Cache + probeErr error + probes int +} + +func (c *coldSnapshotProbeCache) AuthoritativeStat(ctx context.Context, key cache.Key, options ...cache.Option) (http.Header, error) { + c.probes++ + if c.probes <= 2 { + return nil, c.probeErr + } + return c.Cache.Stat(ctx, key, options...) +} + +type snapshotScheduleRecorder struct { + jobscheduler.Scheduler + jobs chan string +} + +func (s *snapshotScheduleRecorder) SubmitPeriodicJob(_, id string, _ time.Duration, _ func(context.Context) error) { + s.jobs <- id +} + +type coldSnapshotSlowCache struct { + cache.Cache + beforeCreate func() error +} + +func (c *coldSnapshotSlowCache) Create(ctx context.Context, key cache.Key, headers http.Header, ttl time.Duration, options ...cache.Option) (cache.Writer, error) { + if err := c.beforeCreate(); err != nil { + return nil, err + } + return c.Cache.Create(ctx, key, headers, ttl, options...) +} + +func TestColdSnapshotDefersInitialBaseJob(t *testing.T) { + for _, publishErr := range []error{nil, errors.New("upload failed")} { + t.Run(fmt.Sprint(publishErr), func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ctx := logging.ContextWithLogger(t.Context(), slog.Default()) + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: t.TempDir()}, nil) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, "https://example.test/org/repo") + assert.NoError(t, err) + for _, args := range [][]string{ + {"init", repo.Path()}, + {"-C", repo.Path(), "-c", "user.name=Test", "-c", "user.email=test@example.com", + "commit", "--allow-empty", "--no-gpg-sign", "-m", "initial"}, + } { + output, err := exec.CommandContext(ctx, "git", args...).CombinedOutput() + assert.NoError(t, err, string(output)) + } + repo.MarkReady() + scheduler := &snapshotScheduleRecorder{jobs: make(chan string, 3)} + mem, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + s := &Strategy{ + scheduler: scheduler, cloneManager: manager, metrics: newGitMetrics(), + config: Config{SnapshotInterval: time.Hour, ZstdThreads: 1}, + } + s.cache = &coldSnapshotSlowCache{Cache: mem, beforeCreate: func() error { + s.scheduleSnapshotJobs(repo) + time.Sleep(2 * time.Hour) + synctest.Wait() + assert.Equal(t, 0, len(scheduler.jobs)) + return publishErr + }} + s.mirrorPreparations.Store(repo.UpstreamURL(), true) + err = s.prepareColdSnapshot(ctx, repo) + if publishErr != nil { + assert.True(t, errors.Is(err, publishErr)) + } else { + assert.NoError(t, err) + _, err = mem.Stat(ctx, snapshotCacheKey(repo.UpstreamURL())) + assert.NoError(t, err) + } + s.mirrorPreparations.Delete(repo.UpstreamURL()) + synctest.Wait() + assert.Equal(t, 2, len(scheduler.jobs)) + assert.Equal(t, snapshotJobLFS+"-periodic", <-scheduler.jobs) + assert.Equal(t, snapshotJobMirror+"-periodic", <-scheduler.jobs) + time.Sleep(time.Hour - time.Nanosecond) + synctest.Wait() + assert.Equal(t, 0, len(scheduler.jobs)) + time.Sleep(time.Nanosecond) + synctest.Wait() + assert.Equal(t, 1, len(scheduler.jobs)) + assert.Equal(t, snapshotJobBase+"-periodic", <-scheduler.jobs) + }) + }) + } +} + +func TestColdSnapshotSkipPreservesCompletion(t *testing.T) { + for _, probeErr := range []error{os.ErrNotExist, errors.New("stat unavailable")} { + t.Run(probeErr.Error(), func(t *testing.T) { + ctx := logging.ContextWithLogger(t.Context(), slog.Default()) + const upstream = "https://example.test/org/repo" + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: t.TempDir()}, nil) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, upstream) + assert.NoError(t, err) + for _, args := range [][]string{ + {"init", repo.Path()}, + {"-C", repo.Path(), "-c", "user.name=Test", "-c", "user.email=test@example.com", + "commit", "--allow-empty", "--no-gpg-sign", "-m", "initial"}, + } { + output, err := exec.CommandContext(ctx, "git", args...).CombinedOutput() + assert.NoError(t, err, string(output)) + } + head, err := exec.CommandContext(ctx, "git", "-C", repo.Path(), "rev-parse", "HEAD").Output() + assert.NoError(t, err) + commit := strings.TrimSpace(string(head)) + repo.MarkReady() + + clock := time.Date(2026, 5, 5, 12, 0, 0, 0, time.UTC) + coord := newTestSnapshotCoordinators(t, func() time.Time { return clock }, 1)[0] + assert.NoError(t, coord.Complete(snapshotJobBase, upstream, commit)) + completedAt := clock + clock = clock.Add(30 * time.Minute) + mem, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + assert.NoError(t, cache.WriteFunc(ctx, mem, snapshotCacheKey(upstream), nil, time.Hour, func(w io.Writer) error { + _, err := io.WriteString(w, "existing snapshot") + return err + })) + c := &coldSnapshotProbeCache{Cache: mem, probeErr: probeErr} + s := &Strategy{ + config: Config{SnapshotInterval: time.Hour, SnapshotMaxAge: time.Hour}, + cache: c, + cloneManager: manager, + snapshotCoord: coord, + metrics: newGitMetrics(), + coldPreparationDelay: func() time.Duration { return 0 }, + } + s.snapshotJobsScheduled.Store(upstream, true) + assert.NoError(t, s.prepareColdSnapshot(ctx, repo)) + assert.Equal(t, 3, c.probes) + rec, ok := coord.gens.Get(snapshotGenKey(snapshotJobBase, upstream)) + assert.True(t, ok) + assert.Equal(t, completedAt, rec.CompletedAt) + assert.Equal(t, clock, rec.CheckedAt) + assert.Zero(t, rec.ClaimID) + assert.True(t, coord.Unchanged(snapshotJobBase, upstream, commit, time.Hour)) + clock = completedAt.Add(time.Hour) + assert.False(t, coord.Unchanged(snapshotJobBase, upstream, commit, time.Hour)) + }) + } +} + func TestSnapshotCoordinatorNilSafe(t *testing.T) { var c *SnapshotCoordinator claimed, err := c.Claim("snapshot", "https://github.com/foo/bar", time.Hour) @@ -32,11 +193,66 @@ func TestSnapshotCoordinatorNilSafe(t *testing.T) { assert.True(t, claimed) assert.NoError(t, c.Complete("snapshot", "https://github.com/foo/bar", "abc123")) assert.NoError(t, c.Skip("snapshot", "https://github.com/foo/bar")) + assert.NoError(t, c.SkipClaim(context.Background(), "snapshot", "https://github.com/foo/bar", "claim")) + assert.NoError(t, c.Fail(context.Background(), "snapshot", "https://github.com/foo/bar", "claim")) assert.False(t, c.Unchanged("snapshot", "https://github.com/foo/bar", "abc123", time.Hour)) assert.NoError(t, c.Prime(context.Background())) assert.Zero(t, NewSnapshotCoordinator(nil)) } +func TestSnapshotCoordinatorFailedClaimCanRetryImmediately(t *testing.T) { + clock := time.Date(2026, 5, 5, 12, 0, 0, 0, time.UTC) + coords := newTestSnapshotCoordinators(t, func() time.Time { return clock }, 2) + const upstream = "https://github.com/example/repo" + + claimID, claimed, err := coords[0].ClaimWithTTL(snapshotJobBase, upstream, snapshotClaimTTL, time.Hour) + assert.NoError(t, err) + assert.True(t, claimed) + assert.NotZero(t, claimID) + assert.NoError(t, coords[0].Fail(context.Background(), snapshotJobBase, upstream, claimID)) + + claimed, err = coords[1].Claim(snapshotJobBase, upstream, snapshotClaimTTL) + assert.NoError(t, err) + assert.True(t, claimed) +} + +func TestSnapshotCoordinatorOwnedClaimSurvivesStaleRelease(t *testing.T) { + clock := time.Date(2026, 5, 5, 12, 0, 0, 0, time.UTC) + coords := newTestSnapshotCoordinators(t, func() time.Time { return clock }, 3) + const upstream = "https://github.com/example/repo" + + firstID, claimed, err := coords[0].ClaimWithTTL(snapshotJobBase, upstream, 0, time.Hour) + assert.NoError(t, err) + assert.True(t, claimed) + clock = clock.Add(time.Hour) + secondID, claimed, err := coords[1].ClaimWithTTL(snapshotJobBase, upstream, 0, time.Hour) + assert.NoError(t, err) + assert.True(t, claimed) + assert.NotEqual(t, firstID, secondID) + + assert.NoError(t, coords[0].Fail(context.Background(), snapshotJobBase, upstream, firstID)) + assert.NoError(t, coords[0].CompleteClaim(context.Background(), snapshotJobBase, upstream, firstID, "stale")) + assert.NoError(t, coords[0].SkipClaim(context.Background(), snapshotJobBase, upstream, firstID)) + _, claimed, err = coords[2].ClaimWithTTL(snapshotJobBase, upstream, 0, time.Hour) + assert.NoError(t, err) + assert.False(t, claimed) + assert.NoError(t, coords[1].CompleteClaim(context.Background(), snapshotJobBase, upstream, secondID, "abc123")) +} + +func TestSnapshotCoordinatorCustomClaimTTL(t *testing.T) { + clock := time.Date(2026, 5, 5, 12, 0, 0, 0, time.UTC) + coords := newTestSnapshotCoordinators(t, func() time.Time { return clock }, 2) + const upstream = "https://github.com/example/repo" + + _, claimed, err := coords[0].ClaimWithTTL(snapshotJobBase, upstream, 0, 2*time.Hour) + assert.NoError(t, err) + assert.True(t, claimed) + clock = clock.Add(snapshotClaimTTL) + claimed, err = coords[1].Claim(snapshotJobBase, upstream, 0) + assert.NoError(t, err) + assert.False(t, claimed) +} + func TestSnapshotCoordinatorUnchanged(t *testing.T) { clock := time.Date(2026, 5, 5, 12, 0, 0, 0, time.UTC) coords := newTestSnapshotCoordinators(t, func() time.Time { return clock }, 2) diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index f745dba0..a91e026d 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -28,6 +28,8 @@ type privateFetcher struct { cloneManager *gitclone.Manager } +const privateCloneWaitTimeout = 30 * time.Minute + type moduleInfo struct { Version string `json:"Version"` Time string `json:"Time"` @@ -125,29 +127,10 @@ func (p *privateFetcher) ensureReady(ctx context.Context, repo *gitclone.Reposit return nil } - if err := repo.Clone(ctx); err != nil { + if err := repo.CloneWithWaitTimeout(ctx, privateCloneWaitTimeout); err != nil { return errors.Wrap(err, "clone repository") } - - ticker := time.NewTicker(100 * time.Millisecond) - defer ticker.Stop() - - timeout := time.After(30 * time.Minute) // reasonable timeout for cloning - - for { - if repo.State() == gitclone.StateReady { - return nil - } - - select { - case <-ticker.C: - // Continue polling - case <-timeout: - return errors.Errorf("timeout waiting for repository %s to be ready", repo.UpstreamURL()) - case <-ctx.Done(): - return errors.Wrap(ctx.Err(), "context cancelled while waiting for clone") - } - } + return nil } func (p *privateFetcher) modulePathToGitURL(modulePath string) string {