Skip to content

feat: redis storage provider and cache deletion jobs (MAPCO-11263) - #41

Open
almog8k wants to merge 14 commits into
masterfrom
feat/redis-storage-provider-MAPCO-11263
Open

almog8k wants to merge 14 commits into
masterfrom
feat/redis-storage-provider-MAPCO-11263

Conversation

@almog8k

@almog8k almog8k commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator
Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Related issues: MAPCO-11263 (epic MAPCO-11261)

Further information:

Adds Redis as a third storage provider and wires the cache-invalidation jobs overseer creates after an ingestion finalizes.

Redis storage provider

  • RedisStorageProvider implementing delete (tile keys via multi-key UNLINK) and deleteResources (prefix wipe via SCAN MATCH {prefix}-*). targetExists is deliberately omitted: a cold prefix is a legitimate empty cache, not a missing target.
  • Connection opened at startup with fail-fast, closed in the onSignal hook. Config under storage.redis, env REDIS_*.
  • DeleteResult.deletedCount is now reported by providers so a Redis run where nothing existed is visible instead of inferred as fully deleted.
  • Tile keys follow the observed mapproxy format {prefix}-{z}-{x}-{y}, no extension.

Cache deletion jobs

  • Update_Delete_Cache + tiles-deletionTilesDeletionStrategy (range deletion over the ingested footprint).
  • Swap_Delete_Cache + tiles-deletionDeleteStoredResourcesStrategy (whole prefix wipe).
  • DeleteStoredResourcesStrategy honors delaySeconds on Redis params so a swap wipe waits for the mapproxy reload window before removing keys. The queue heartbeat keeps the task alive during the wait.
  • REDIS is enabled by default in cleanupStorageProviders, since the capability pairs are on by default.

Helm

  • storage.redis mirrors the global.redis hierarchy and merges over global.storage.redis and global.redis.
  • cleanupStorageProviders now defaults to a list and env.jobnik.worker.concurrency gets its default, so the chart renders without site overrides.

Tests

  • Unit coverage for the provider, connection, config, strategies and the delaySeconds wait.
  • Integration tests against a Redis testcontainer (alongside the existing Minio one). A follow-up PR consolidates the integration suites of both strategies onto shared backends and helpers.

Still needed outside this repo

  • job-tracker rejects unknown job types, so the cache-deletion job is never finalized after its last task acks. Needs a handler for the two new job types.
  • Job definitions and REDIS in cleanupStorageProviders for the target environments (done locally in site-values, not yet pushed).

Adds the ioredis dependency and the storage.redis config block, following
the existing buildFsStorageConfig / buildS3StorageConfig pattern: a single
place where the untyped config.get() cast happens, the delete.batchSize is
hoisted flat, and bad values fail at boot rather than on the first task.

scanCount is the SCAN COUNT hint. It is validated as > 0 because Redis
rejects COUNT 0 with 'ERR syntax error' rather than falling back to a
default, which would otherwise fail every prefix wipe at task time.

Credentials are logged field by field rather than spread, so a configured
password never reaches the logs.

REDIS is deliberately left out of the default cleanupStorageProviders:
startup is fail-fast, so enabling it by default would impose a hard Redis
dependency before any task can dispatch to it (CL-3).
Opens the Redis connection at startup and returns ioredis's client directly,
the way S3StorageProvider uses the SDK's own S3Client.

Unlike the S3 and FS clients, which are stateless and built inside their
providers, this one is a long-lived socket. It is created outside the provider
so a bad host or credential fails the service at boot alongside the other
storage config, and so the onSignal hook has something to close.

Standalone only, by design. There is no topology probe: a directly sharded
cluster cannot be populated by the serving side in the first place, so a sweep
there finds nothing rather than under-deleting something, and a proxy-fronted
shard reports cluster_enabled:0 and would evade a probe anyway. The general
detector for 'we deleted nothing' is the deleted-count signal, which lands with
the provider.
Implements delete() for REDIS: keys are chunked by the configured batch size
and each chunk goes out as a single multi-key UNLINK, summing the integer
replies. deleteResources() and targetExists() are stubbed for the next change.

DeleteResult gains an optional deletedCount. S3 raises NoSuchKey and FS raises
ENOENT on a miss, so their callers can infer what was removed from the failures.
UNLINK on an absent key returns 0 without raising, so without an observed count
a completely wrong prefix would infer full success.

A chunk that throws is recorded whole against its reason, with the first key as
the sample: the command is atomic, so nothing in it was removed, and Redis gives
no per-key attribution to do better.
Implements deleteResources() as a prefix wipe: scanKeys pages through SCAN and
hands each non-empty page to unlinkInBatches, merging failures and summing
counts across pages.

scanKeys ends on the cursor returning to '0', never on an empty page. MATCH
filters after the elements are retrieved, so a page can come back empty while
keys still remain further along the keyspace.

targetExists reports whether any key lives under the prefix, short-circuiting on
the first non-empty page. It ignores relativePath, since a Redis prefix has no
sub paths. Note it has no caller once the strategy is unblocked for REDIS: an
empty prefix there is a legitimate cold cache rather than a missing target, so
the strategy skips the precondition.

unlinkInBatches now returns a required deletedCount rather than a DeleteResult,
which drops an unreachable '?? 0' fallback in deleteResources.
Make deletedCount required on DeleteResult, propagate it from the FS and S3
providers through the tiles-deletion strategy, and log it in the outcome.
Extract countFailures and trim comments.
Make targetExists optional on IStorageProvider and drop it from the Redis
provider, since an empty prefix is a cold cache rather than a missing target.
…O-11263)

Register Update_Delete_Cache and Swap_Delete_Cache with the tiles-deletion task,
mapped to TilesDeletionStrategy and DeleteStoredResourcesStrategy respectively,
matching the jobs overseer's CacheDeletionJobCreator emits.

DeleteStoredResourcesStrategy honors delaySeconds on Redis params so a swap wipe
waits for the mapproxy reload window before removing keys.

Enable the REDIS storage provider by default alongside FS and S3.
…airs (MAPCO-11263)

storage.redis follows the global.redis hierarchy and merges over
global.storage.redis and global.redis. REDIS_* env is emitted when REDIS is in
cleanupStorageProviders.

Also default cleanupStorageProviders to a list and add the env.jobnik.worker
default the configmap already reads, so the chart renders without overrides.
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

🎫 Related Jira Issue: MAPCO-11263

…kends (MAPCO-11263)

Run TilesDeletionStrategy and DeleteStoredResourcesStrategy through the real
poller against Minio, a temp filesystem and a Redis testcontainer, each under the
job it is registered for in production (Update_Delete_Cache / Swap_Delete_Cache
for Redis).

Both suites share one backend lifecycle, provider construction, task runner and
outcome assertions, with per-strategy adapters layered on top. The standalone
Redis provider spec is folded into the strategy suites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant