Pr/sync redis heartbeat by mrudnoru · Pull Request #4099 · project-zot/zot

mrudnoru · 2026-05-25T19:44:36Z

What type of PR is this?

feature

Which issue does this PR fix:

N/A

What does this PR do / Why do we need it:

Adds distributed coordination for the on-demand sync path so that, in multi-replica deployments, only one replica runs a given image sync at a time; concurrent requests on other replicas return 503 + Retry-After.

Mechanics:

New RedisDistributedLock wraps go-redis SetNX / EVAL for TryLock / Unlock / Refresh / IsLocked. Refresh uses a Lua script that only extends TTL when the caller still owns the key (CAS semantics).
BaseOnDemand gains an optional distributedLockBackend; when set, SyncImage and SyncReferrers acquire a key derived from sha256(kind | repo | reference) before delegating to the existing in-process syncImage / syncReferrers.
A heartbeat goroutine refreshes the lock TTL every 30s with a fixed 90s TTL, so a single missed heartbeat does not drop the lock and a dead leader's lock expires within ~90s.
New IsSyncInFlight on SyncOnDemand lets the manifest and referrers routes short-circuit to 503 when another replica holds the lock, rather than starting a parallel upstream sync.

Configuration:

The lock backend is auto-enabled when storage.cacheDriver.name == "redis"; no extra config required. Single-replica / in-memory cache deployments keep the prior in-process dedup behavior unchanged.
New sync.asyncManifest (bool) makes manifest GET kick the sync off in the background and return 503 immediately — useful when the client (e.g. kubelet) retries faster than upstream pulls complete. Validated to require at least one onDemand sync registry, otherwise ErrBadConfig.

See examples/config-sync-multireplica.json for a complete configuration.

The existing in-process requestStore dedup is preserved for the fast case (two parallel kubelet retries hitting the same replica).

If an issue # is not available please add repro steps and logs showing the issue:

Without coordination, N zot replicas behind a load balancer that receive concurrent manifest GETs for the same uncached image will each start an independent upstream pull. Symptoms:

N× upstream bandwidth and N× upstream rate-limit consumption per image.
Race on the final blob/manifest write to shared S3 storage.
N× memory and CPU for the same sync operation.

After this change, the first replica to TryLock runs the sync; other replicas observe IsSyncInFlight == true and return 503 + Retry-After, which kubelet (and most other OCI clients) honor.

Testing done on this change:

$ go test -tags 'sync lint' ./pkg/extensions/sync \
    -run 'TestRedisLock|TestRedisLockCoordination' -count=1
ok    zotregistry.dev/zot/v2/pkg/extensions/sync      ...

Coordination tests use miniredis and cover:

cross-replica dedup (one replica acquires, other observes in-flight)
in-flight detection without holding the lock
no-backend no-op path (single-replica / in-memory cache configs)

Manual validation on a 3-replica deployment backed by S3 + Redis:

Triggered concurrent docker pull of a previously-uncached image across all replicas.
Confirmed exactly one upstream pull initiated, other replicas returned 503 with Retry-After, clients eventually pulled from the shared S3 backend.

Automation added to e2e:

Yes.

test/blackbox/sync_multireplica_lock.bats` exercises the distributed lock end-to-end: it spins up Redis + an upstream zot + two replica zots configured to sync on-demand from upstream against the shared

Redis cache, fires concurrent manifest GETs against both replicas, and asserts:

exactly one replica returns 200 and one returns 503 (the lock holder vs the loser);
the loser's log contains distributed on-demand sync already in flight — the direct signal that the Redis lock fired, not just in-process dedup.

The new suite is wired into test/blackbox/ci.sh so it runs as part of make run-blackbox-ci.

Unit + integration coverage also lives in pkg/extensions/sync/redis_lock_test.go and pkg/extensions/sync/redis_lock_coordination_test.go, using miniredis to drive the same scenarios in a single process for faster feedback.

Will this break upgrades or downgrades?

No.

No new required configuration. The lock backend is auto-enabled when storage.cacheDriver.name == "redis"; deployments without Redis are unaffected.
sync.asyncManifest defaults to false; existing configs keep synchronous manifest behavior.
Downgrade to a pre-feature build releases locks naturally on TTL expiry (~90s) — no orphaned state.

Does this PR introduce any user-facing change?:

Yes. Multi-replica deployments with the Redis cache driver now coordinate on-demand sync across pods. New optional sync.asyncManifest flag.

Multi-replica deployments with storage.cacheDriver.name == "redis" now coordinate on-demand sync across pods using a distributed lock with heartbeat. Concurrent requests on other replicas return 503 + Retry-After instead of triggering parallel upstream pulls. New optional sync.asyncManifest flag returns 503 immediately and runs the sync in the background.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copilot

Pull request overview

This PR adds cross-replica coordination for on-demand sync in multi-replica deployments by introducing a Redis-backed distributed lock (with TTL refresh heartbeat) and wiring API handlers to return 503 + Retry-After when a sync is already in flight. It also introduces an optional sync.asyncManifest mode to kick off background syncs on manifest cache misses while immediately returning 503 to encourage client retries.

Changes:

Add Redis distributed lock implementation (TryLock/Unlock/Refresh/IsLocked) plus unit/integration tests (miniredis-based).
Extend on-demand sync to optionally acquire a distributed lock + run a heartbeat during long syncs; expose IsSyncInFlight() for routes to short-circuit.
Add blackbox CI coverage for multi-replica coordination and config validation for sync.asyncManifest.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/blackbox/sync_multireplica_lock.bats	New end-to-end blackbox test that spins up Redis + upstream zot + two replicas and asserts exactly one 200 vs one 503 outcome.
test/blackbox/ci.sh	Adds the new blackbox test to the CI test matrix.
pkg/extensions/sync/redis_lock.go	Implements a Redis-backed distributed lock using `SETNX` and Lua CAS-style `PEXPIRE`/`DEL`.
pkg/extensions/sync/redis_lock_test.go	Unit tests for lock refresh/ownership semantics with miniredis.
pkg/extensions/sync/redis_lock_coordination_test.go	Integration-style tests validating cross-“replica” dedup and no-backend no-op behavior.
pkg/extensions/sync/on_demand.go	Adds distributed-lock support, heartbeat refresh, and `IsSyncInFlight`; refactors sync entrypoints to run via `runWithLock`.
pkg/extensions/sync/on_demand_disabled.go	Adds stub `IsSyncInFlight` for non-`sync` builds.
pkg/extensions/extension_sync.go	Auto-enables the Redis distributed lock when `storage.cacheDriver.name == "redis"`.
pkg/extensions/config/sync/config.go	Adds `AsyncManifest` config field.
pkg/cli/server/root.go	Validates `sync.asyncManifest` requires at least one `onDemand` registry.
pkg/api/routes.go	Returns `503 + Retry-After` on `ErrSyncInFlight`; supports async-manifest behavior and referrers in-flight handling.
pkg/api/controller.go	Extends `SyncOnDemand` interface with `IsSyncInFlight`.
examples/config-sync-multireplica.json	Adds an example multi-replica config using Redis cache driver + `asyncManifest`.
errors/errors.go	Introduces `ErrSyncInFlight` sentinel error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mrudnoru · 2026-06-11T07:05:21Z

+	handle, release, err := onDemand.acquireDistributedLock(ctx, kind, repo, reference)
+	if err != nil {
+		close(syncResult)

 		return err


Fixed in 47f55ac — replaced the shared chan error with a syncResult struct (done chan struct{} + stored err), so all deduped waiters get broadcast semantics including early failures before the sync goroutine starts (e.g. ErrSyncInFlight now reliably surfaces as 503 to every waiter).

rchincha · 2026-05-25T22:44:13Z

+function verify_prerequisites() {
+    if [ ! $(command -v curl) ]; then
+        echo "you need to install curl as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
+    if [ ! $(command -v docker) ]; then
+        echo "you need to install docker as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
+    if [ ! $(command -v skopeo) ]; then
+        echo "you need to install skopeo as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
+    return 0
+}


the blackbox test comments can be ignored for now

+function setup_file() {
+    if ! $(verify_prerequisites); then
+        exit 1
+    fi


rchincha · 2026-05-26T06:28:52Z

cc: @vrajashkr

rchincha · 2026-05-29T16:14:35Z

@mrudnoru pls continue working on this but also give us some time to work through this. This is a very involved PR.

Adds a distributed locking mechanism for the on-demand sync path so that, in multi-replica deployments, only one replica runs a given image sync at a time; concurrent requests on other replicas return 503 + Retry-After. Mechanics: - New RedisDistributedLock wraps go-redis SetNX/EVAL for TryLock/ Unlock/Refresh/IsLocked. Refresh uses a Lua script that only extends TTL when the caller still owns the key (CAS semantics). - BaseOnDemand gains an optional distributedLockBackend; when set, SyncImage and SyncReferrers acquire a key derived from sha256(kind|repo|reference) before delegating to the existing in-process syncImage/syncReferrers. - A heartbeat goroutine refreshes the lock TTL every 30s with a fixed 90s TTL, so a single missed heartbeat doesn't drop the lock and a dead leader's lock expires within ~90s. - New IsSyncInFlight on SyncOnDemand lets the manifest route short-circuit to 503 when another replica holds the lock, rather than starting a parallel upstream sync. Configuration: - The lock backend is auto-enabled when storage.cacheDriver.name == "redis"; no extra config needed. Single-replica/in-memory cache deployments keep the prior in-process dedup behavior unchanged. - Sync.AsyncManifest (new bool) makes manifest GET kick the sync off in the background and return 503 immediately, useful when the client (e.g. kubelet) retries faster than upstream pulls. The existing in-process requestStore dedup is preserved for the fast case (two parallel kubelet retries hitting the same replica). Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

…back - add IsSyncInFlight stub to the !sync BaseOnDemand so non-sync builds satisfy the SyncOnDemand interface and compile - enforce asyncManifest requires at least one onDemand registry in validateSync, erroring with ErrBadConfig otherwise - add coordination tests covering cross-replica dedup, in-flight detection, and the no-backend no-op path via miniredis Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

- propagate ErrSyncInFlight from getReferrers so the GetReferrers handler returns 503 + Retry-After instead of serving possibly-stale referrers - document IsSyncInFlight as advisory, with TryLock as the authoritative cross-replica dedup - drop redundant restate-the-code comments from the coordination test Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

Mirrors examples/config-sync.json style. Combines Redis cache driver, S3 storage, asyncManifest, and an on-demand sync registry — the minimum config that exercises the distributed lock + heartbeat path introduced in this branch. Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

Spins up 1 Redis + 1 upstream zot + 2 replica zots sharing the Redis cache, fires concurrent manifest GETs against both replicas, asserts exactly one 200 + one 503 and that the loser's log records the distributed in-flight observation. Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

Replace the shared chan error in requestStore with a shared result object so every deduped caller observes the same outcome, including early lock failures that previously delivered a nil (success) result. Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

…ireplica example Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

mrudnoru requested review from andaaron and rchincha as code owners May 25, 2026 19:44

rchincha requested review from Copilot and vrajashkr May 25, 2026 21:55

Copilot started reviewing on behalf of rchincha May 25, 2026 21:55 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

mrudnoru force-pushed the pr/sync-redis-heartbeat branch from 47f55ac to a459e9d Compare June 16, 2026 07:08

mrudnoru added 7 commits June 23, 2026 23:16

fix(sync): resolve golangci-lint findings and add downloadDir to mult…

d326b70

…ireplica example Signed-off-by: Marga Rudno-Rudzińska <marga.rudno@gmail.com>

mrudnoru force-pushed the pr/sync-redis-heartbeat branch from cd8c732 to d326b70 Compare June 23, 2026 21:25

rchincha mentioned this pull request Jul 10, 2026

[Feat]: Improve scale-out #4215

Open

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Pr/sync redis heartbeat#4099

Pr/sync redis heartbeat#4099
mrudnoru wants to merge 7 commits into
project-zot:mainfrom
mrudnoru:pr/sync-redis-heartbeat

mrudnoru commented May 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mrudnoru Jun 11, 2026

Uh oh!

rchincha May 25, 2026

Uh oh!

rchincha commented May 26, 2026

Uh oh!

rchincha commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mrudnoru commented May 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mrudnoru Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

rchincha May 25, 2026

Choose a reason for hiding this comment

Uh oh!

rchincha commented May 26, 2026

Uh oh!

rchincha commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants