Confine phased sync slots to sync_interval remotes

Unset remotes keep the mutable_ttl freshness predicate. Scheduled remotes compute their slot in Go, skip the create-time prime and wait for their first slot; a cold-cache prime still runs at once. Document sync_interval.
2026-10-25 13:27:43 +11:00
parent 707e380c98
commit 1582533e5a
16 changed files with 343 additions and 135 deletions
@@ -1,7 +1,7 @@
 package database

 import (
-	"fmt"
+	"context"
 	"testing"
 	"time"

@@ -31,7 +31,7 @@ func TestGitHubSyncLease(t *testing.T) {
 	freshness := time.Hour

 	// First claim on a never-synced remote wins; etag starts empty.
-	claimed, etag, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-1", freshness, lease)
+	claimed, etag, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-1", freshness, lease, time.Time{})
 	if err != nil || !claimed {
 		t.Fatalf("replica-1 first claim: claimed=%v err=%v", claimed, err)
 	}
@@ -40,7 +40,7 @@ func TestGitHubSyncLease(t *testing.T) {
 	}

 	// A second replica cannot claim while the lease is held.
-	claimed2, _, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", freshness, lease)
+	claimed2, _, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", freshness, lease, time.Time{})
 	if err != nil {
 		t.Fatalf("replica-2 claim err: %v", err)
 	}
@@ -54,7 +54,7 @@ func TestGitHubSyncLease(t *testing.T) {
 	}

 	// A periodic re-claim inside the freshness window is blocked by recency.
-	claimed3, _, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", freshness, lease)
+	claimed3, _, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", freshness, lease, time.Time{})
 	if err != nil {
 		t.Fatalf("replica-2 recency claim err: %v", err)
 	}
@@ -63,7 +63,7 @@ func TestGitHubSyncLease(t *testing.T) {
 	}

 	// A prime (freshness 0) bypasses recency and reads the persisted etag.
-	claimed4, etag4, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", 0, lease)
+	claimed4, etag4, err := testDB.ClaimGitHubSyncLease(ctx(), name, "replica-2", 0, lease, time.Time{})
 	if err != nil || !claimed4 {
 		t.Fatalf("prime claim: claimed=%v err=%v", claimed4, err)
 	}
@@ -109,7 +109,7 @@ func TestGitHubSyncLeaseFailureBackoff(t *testing.T) {
 	freshness := time.Hour
 	claim := func(f time.Duration) (bool, string) {
 		t.Helper()
-		ok, etag, err := testDB.ClaimGitHubSyncLease(ctx(), name, "r1", f, lease)
+		ok, etag, err := testDB.ClaimGitHubSyncLease(ctx(), name, "r1", f, lease, time.Time{})
 		if err != nil {
 			t.Fatalf("claim: %v", err)
 		}
@@ -221,21 +221,21 @@ func TestGitHubSyncStateDeletedWithRemote(t *testing.T) {
 	}{
 		{"github_rpm_sync_state", models.PackageGitHubRPM,
 			func(n string) (bool, string, error) {
-				return testDB.ClaimGitHubSyncLease(ctx(), n, "r1", time.Hour, time.Minute)
+				return testDB.ClaimGitHubSyncLease(ctx(), n, "r1", time.Hour, time.Minute, time.Time{})
 			},
 			func(n string, res provider.SyncResult) error {
 				return testDB.ReleaseGitHubSyncLease(ctx(), n, "r1", res)
 			}},
 		{"github_deb_sync_state", models.PackageGitHubDeb,
 			func(n string) (bool, string, error) {
-				return testDB.ClaimGitHubDebSyncLease(ctx(), n, "r1", time.Hour, time.Minute)
+				return testDB.ClaimGitHubDebSyncLease(ctx(), n, "r1", time.Hour, time.Minute, time.Time{})
 			},
 			func(n string, res provider.SyncResult) error {
 				return testDB.ReleaseGitHubDebSyncLease(ctx(), n, "r1", res)
 			}},
 		{"github_alpine_sync_state", models.PackageGitHubAlpine,
 			func(n string) (bool, string, error) {
-				return testDB.ClaimGitHubAlpineSyncLease(ctx(), n, "r1", time.Hour, time.Minute)
+				return testDB.ClaimGitHubAlpineSyncLease(ctx(), n, "r1", time.Hour, time.Minute, time.Time{})
 			},
 			func(n string, res provider.SyncResult) error {
 				return testDB.ReleaseGitHubAlpineSyncLease(ctx(), n, "r1", res)
@@ -278,85 +278,95 @@ func TestGitHubSyncStateDeletedWithRemote(t *testing.T) {
 	}
 }

-func TestSyncPhaseStableAndSpread(t *testing.T) {
-	const period = 5 * time.Minute
-	phases := map[time.Duration]bool{}
-	var lo, hi time.Duration = period, 0
-	for i := 0; i < 13; i++ {
-		name := fmt.Sprintf("github-rpm-%02d", i)
-		p := syncPhase(name, period)
-		if p != syncPhase(name, period) {
-			t.Fatalf("%s: phase not stable", name)
+type syncTable struct {
+	table string
+	pt    models.PackageType
+	claim func(name, owner string, freshness time.Duration, slot time.Time) (bool, error)
+}
+
+func syncTables() []syncTable {
+	wrap := func(fn func(context.Context, string, string, time.Duration, time.Duration, time.Time) (bool, string, error)) func(string, string, time.Duration, time.Time) (bool, error) {
+		return func(name, owner string, freshness time.Duration, slot time.Time) (bool, error) {
+			ok, _, err := fn(ctx(), name, owner, freshness, 15*time.Minute, slot)
+			return ok, err
 		}
-		if p < 0 || p >= period {
-			t.Fatalf("%s: phase %v outside [0,%v)", name, p, period)
-		}
-		phases[p.Truncate(time.Second)] = true
-		lo, hi = min(lo, p), max(hi, p)
 	}
-	if len(phases) != 13 {
-		t.Fatalf("phases collide: %d distinct of 13", len(phases))
-	}
-	if hi-lo < period/2 {
-		t.Fatalf("phases bunched in %v of %v", hi-lo, period)
-	}
-	if syncPhase("x", 0) != 0 {
-		t.Fatal("zero period must give zero phase")
+	return []syncTable{
+		{"github_rpm_sync_state", models.PackageGitHubRPM, wrap(testDB.ClaimGitHubSyncLease)},
+		{"github_deb_sync_state", models.PackageGitHubDeb, wrap(testDB.ClaimGitHubDebSyncLease)},
+		{"github_alpine_sync_state", models.PackageGitHubAlpine, wrap(testDB.ClaimGitHubAlpineSyncLease)},
 	}
 }

-// TestGitHubSyncLeasePhasedSlots checks a remote is due once per slot: synced
-// before its latest boundary (or over a TTL ago) it is claimable by exactly one
-// replica; synced after the boundary it is not.
-func TestGitHubSyncLeasePhasedSlots(t *testing.T) {
+// seedSynced creates a remote of tc's type with a released lease last synced at.
+func seedSynced(t *testing.T, tc syncTable, prefix string, at time.Time) string {
+	t.Helper()
+	name := prefix + "-" + tc.table + "-" + time.Now().Format("150405.000000")
+	if err := testDB.CreateRemote(ctx(), &models.Remote{
+		Name: name, PackageType: tc.pt, RepoType: models.RepoTypeRemote,
+		BaseURL: "https://api.github.com/repos/acme/tools", ReleasesRemote: "github",
+	}); err != nil {
+		t.Fatalf("create remote: %v", err)
+	}
+	if _, err := testDB.Pool.Exec(ctx(), `INSERT INTO `+tc.table+` (remote_name, last_synced_at) VALUES ($1, $2)`, name, at); err != nil {
+		t.Fatalf("seed sync state: %v", err)
+	}
+	return name
+}
+
+func mustClaim(t *testing.T, tc syncTable, name, owner string, freshness time.Duration, slot time.Time) bool {
+	t.Helper()
+	ok, err := tc.claim(name, owner, freshness, slot)
+	if err != nil {
+		t.Fatalf("claim: %v", err)
+	}
+	return ok
+}
+
+// With no slot the claim keeps the freshness-since-last-sync predicate.
+func TestSyncLeaseUnsetIntervalFreshness(t *testing.T) {
 	requireDB(t)
-	const lease = 15 * time.Minute
-	ttl := time.Hour
-
-	setSynced := func(name string, at time.Time) {
-		t.Helper()
-		if _, err := testDB.Pool.Exec(ctx(), `UPDATE github_rpm_sync_state SET last_synced_at = $2, sync_lease_owner = '', sync_lease_expires = NULL WHERE remote_name = $1`, name, at); err != nil {
-			t.Fatalf("set last_synced_at: %v", err)
-		}
-	}
-	claim := func(name, owner string) bool {
-		t.Helper()
-		ok, _, err := testDB.ClaimGitHubSyncLease(ctx(), name, owner, ttl, lease)
-		if err != nil {
-			t.Fatalf("claim: %v", err)
-		}
-		return ok
-	}
-
-	for i := 0; i < 5; i++ {
-		name := fmt.Sprintf("gh-slot-%d-%s", i, time.Now().Format("150405.000000"))
-		seedGitHubRPMRemote(t, name)
-		if !claim(name, "replica-1") {
-			t.Fatalf("%s: first claim failed", name)
-		}
-
-		now := time.Now()
-		phase := syncPhase(name, ttl)
-		boundary := now.Add(-phase).Truncate(ttl).Add(phase)
-
-		setSynced(name, now.Add(-ttl-time.Second))
-		if !claim(name, "replica-1") {
-			t.Fatalf("%s: not due after a full TTL", name)
-		}
-		if claim(name, "replica-2") {
-			t.Fatalf("%s: second replica claimed a held lease", name)
-		}
-
-		setSynced(name, boundary.Add(-5*time.Second))
-		if !claim(name, "replica-2") {
-			t.Fatalf("%s: not due after crossing its boundary %v", name, boundary)
-		}
-
-		if now.Sub(boundary) > 10*time.Second {
-			setSynced(name, boundary.Add(5*time.Second))
-			if claim(name, "replica-1") {
-				t.Fatalf("%s: re-synced within the same slot", name)
+	const ttl = time.Hour
+	for _, tc := range syncTables() {
+		t.Run(tc.table, func(t *testing.T) {
+			for _, c := range []struct {
+				ago       time.Duration
+				freshness time.Duration
+				due       bool
+			}{
+				{ttl + time.Minute, ttl, true},
+				{ttl - time.Minute, ttl, false},
+				{time.Second, ttl, false},
+				{time.Second, 0, true},
+			} {
+				name := seedSynced(t, tc, "fresh", time.Now().Add(-c.ago))
+				if got := mustClaim(t, tc, name, "r1", c.freshness, time.Time{}); got != c.due {
+					t.Errorf("synced %v ago, freshness %v: claimed=%v, want %v", c.ago, c.freshness, got, c.due)
+				}
 			}
-		}
+		})
+	}
+}
+
+// With a slot the remote is due only when last synced before it, regardless of
+// freshness, and still one replica at a time.
+func TestSyncLeaseSlot(t *testing.T) {
+	requireDB(t)
+	slot := time.Date(2026, 3, 1, 10, 17, 42, 0, time.UTC)
+	for _, tc := range syncTables() {
+		t.Run(tc.table, func(t *testing.T) {
+			name := seedSynced(t, tc, "slot", slot.Add(time.Second))
+			if mustClaim(t, tc, name, "r1", 0, slot) {
+				t.Fatal("claimed though synced inside the slot")
+			}
+
+			name = seedSynced(t, tc, "slot", slot.Add(-time.Second))
+			if !mustClaim(t, tc, name, "r1", time.Hour*24*365*10, slot) {
+				t.Fatal("not due though last synced before the slot")
+			}
+			if mustClaim(t, tc, name, "r2", 0, slot) {
+				t.Fatal("second replica claimed a held lease")
+			}
+		})
 	}
 }