Skip to content

software: tune redis as in-memory LRU cache with RDB-on-shutdown#940

Open
igorpecovnik wants to merge 2 commits into
mainfrom
software/redis-lru-cache
Open

software: tune redis as in-memory LRU cache with RDB-on-shutdown#940
igorpecovnik wants to merge 2 commits into
mainfrom
software/redis-lru-cache

Conversation

@igorpecovnik

@igorpecovnik igorpecovnik commented Jun 11, 2026

Copy link
Copy Markdown
Member

Reconfigures module_redis for use as the ccache remote-storage backend (CCACHE_REMOTE_STORAGE): many build runners moving large object blobs over a fast link, with allkeys-lru evicting old objects.

Base config

docker run -d --name redis --net=lsio --restart=always \
  -p 6379:6379 --ulimit nofile=100000:100000 \
  --stop-timeout 300 -v /armbian/redis/data:/data \
  redis:latest redis-server \
    --maxmemory 64gb --maxmemory-policy allkeys-lru --maxmemory-samples 10 \
    --io-threads 4 --io-threads-do-reads yes \
    --lazyfree-lazy-eviction yes --lazyfree-lazy-expire yes --lazyfree-lazy-server-del yes \
    --activedefrag yes \
    --appendonly no --save "86400 1"

Cache behavior

  • Image redis:alpineredis:latest; maxmemory 64gb + allkeys-lru (bounded, leaves headroom for RDB-fork COW and OS page cache on a 128 GB host)
  • Persistence: AOF off, RDB via an infrequent save point (86400 1) → snapshot on graceful shutdown, reloaded on start; /data volume; --stop-timeout 300 so a large snapshot finishes before SIGKILL

Throughput tuning (ccache workload)

  • io-threads 4 + io-threads-do-reads — parallelize network read/write (the bottleneck for big blobs over a fat pipe; reads matter since cache hits pull objects)
  • lazyfree-lazy-{eviction,expire,server-del} — free large evicted values on a background thread so the event loop never stalls when full
  • maxmemory-samples 10 — more accurate LRU
  • activedefrag yes — reclaim fragmentation in a long-running cache

Tunables (maxmemory, io_threads, maxmemory_samples, save, stop_timeout, nofile) are exposed as module_options. Kept --net=lsio, -p 6379:6379, --restart=always as-is.

Note: --stop-timeout covers docker stop; a host reboot instead uses the Docker service's systemd TimeoutStopSec.

Reconfigure the redis module for cache use: redis:latest, maxmemory
64gb with allkeys-lru eviction, raised nofile ulimit, and AOF off. RDB
persistence kept via an infrequent save point (86400 1) so a snapshot is
written on graceful shutdown and reloaded on start; --stop-timeout 300
lets a large snapshot finish before SIGKILL. Network, published port and
restart policy unchanged. Tunables exposed as module_options; help and
REDIS1 header updated to match.

Signed-off-by: Igor Pecovnik <igor@armbian.com>
@github-actions github-actions Bot added size/small PR with less then 50 lines 08 Milestone: Third quarter release Documentation Documentation changes or additions labels Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR enhances the Redis module configuration by adding configurable options for memory limits, eviction policy, file descriptor limits, RDB persistence, and graceful shutdown. The module metadata declares five new options (maxmemory, maxmemory_policy, nofile, save, stop_timeout), switches the default Docker image from redis:alpine to redis:latest, and updates the module description. The implementation layer extends the docker run invocation to read these options and apply them as container ulimits, Docker flags, and redis-server arguments. Module documentation is revised to describe Redis as an in-memory LRU cache with RDB persistence, and the help output is expanded to display the new configuration parameters to users.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: tuning Redis as an in-memory LRU cache with RDB-on-shutdown persistence, which is the core objective of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively explains the reconfiguration of module_redis as an in-memory LRU cache backend, detailing the base config, cache behavior, and throughput tuning rationale.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch software/redis-lru-cache

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Redis backs ccache remote storage: many runners moving large object
blobs over a fast link, with allkeys-lru evicting old objects. Add:
  - io-threads 4 (+io-threads-do-reads) — parallelize network read/write
  - lazyfree-lazy-{eviction,expire,server-del} — non-blocking eviction
    of large values so the event loop never stalls when full
  - maxmemory-samples 10 — more accurate LRU
  - activedefrag yes — reclaim fragmentation in a long-running cache

io-threads and maxmemory-samples are exposed as module_options. Help and
REDIS1 header updated.

Signed-off-by: Igor Pecovnik <igor@armbian.com>
@github-actions github-actions Bot added size/medium PR with more then 50 and less then 250 lines and removed size/small PR with less then 50 lines labels Jun 11, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
tools/include/markdown/REDIS1-header.md (1)

2-2: ⚡ Quick win

Add the host-reboot timeout caveat to avoid over-promising durability.

Lines 2/6 describe planned restart persistence, but reboot behavior also depends on the Docker service’s systemd TimeoutStopSec (not just container --stop-timeout). Add a short caveat here for operator clarity.

Also applies to: 6-6, 11-11

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/include/markdown/REDIS1-header.md` at line 2, The sentence describing
"in-memory LRU cache with RDB persistence" overstates durability for reboots;
update that line (and the similar lines at 6 and 11) to add a short caveat that
RDB snapshots are written on graceful shutdown but a host-level reboot may still
interrupt the container before the RDB is flushed if the host's systemd
TimeoutStopSec is shorter than Docker's --stop-timeout, so operators should
ensure systemd TimeoutStopSec is adjusted or use additional persistence
strategies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tools/modules/software/module_redis.sh`:
- Line 14: The default Redis maxmemory value is set too high
("module_redis,maxmemory" => "64gb"); change it to a safe conservative default
(e.g., "1gb") or make it unset so admins supply it, and update the other
occurrence of the same key as well; locate the setting identified by
"module_redis,maxmemory" in module_redis.sh and replace "64gb" with the new
safer default (or remove the default) so small hosts won't be driven to OOM by
Redis.
- Line 12: The Docker image for Redis is pinned to the mutable tag in the
setting ["module_redis,dockerimage"]="redis:latest"; replace that value with a
fixed, specific version tag (e.g. a minor/patch like redis:7.2.1) or an
immutable image digest to ensure reproducible installs and avoid pulling
different builds over time. Update the value for the
["module_redis,dockerimage"] entry and, if possible, include the exact patch or
digest you want the deployment to use.

---

Nitpick comments:
In `@tools/include/markdown/REDIS1-header.md`:
- Line 2: The sentence describing "in-memory LRU cache with RDB persistence"
overstates durability for reboots; update that line (and the similar lines at 6
and 11) to add a short caveat that RDB snapshots are written on graceful
shutdown but a host-level reboot may still interrupt the container before the
RDB is flushed if the host's systemd TimeoutStopSec is shorter than Docker's
--stop-timeout, so operators should ensure systemd TimeoutStopSec is adjusted or
use additional persistence strategies.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 501c9d8d-7e4c-4547-967c-8600419a1ad5

📥 Commits

Reviewing files that changed from the base of the PR and between ea69013 and 65e664f.

📒 Files selected for processing (2)
  • tools/include/markdown/REDIS1-header.md
  • tools/modules/software/module_redis.sh

["module_redis,port"]="6379"
["module_redis,arch"]="x86-64 arm64"
["module_redis,dockerimage"]="redis:alpine"
["module_redis,dockerimage"]="redis:latest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Is Docker Hub redis:latest a mutable moving tag, and which Redis major/minor tag is currently recommended for production pinning?

💡 Result:

Yes, the redis:latest tag on Docker Hub is a mutable, moving tag [1][2]. It is updated whenever a new version of the Redis image is released, meaning it does not point to a fixed, immutable build [1][3]. Consequently, using redis:latest in production is discouraged as it can lead to inconsistent deployments, unexpected behavior changes, or compatibility issues if a new major version or variant is pulled automatically [4]. For production pinning, it is recommended to use specific version tags [4]. The most common strategies are: 1. Full Patch Version Pinning: Use a specific patch version (e.g., redis:8.8.0 or redis:8.8.0-alpine) to ensure your environment uses an exact, immutable image [3][5]. This is the most secure method for avoiding unexpected changes [4]. 2. Minor Version Pinning: Use a minor version tag (e.g., redis:8.8 or redis:8.8-alpine) if you want to automatically receive backward-compatible patch updates (such as critical security fixes) while avoiding major version upgrades [3][5][4]. In addition to the version number, you should consider the underlying operating system variant (e.g., -trixie for Debian or -alpine) [2][5]. If your production environment requires stability or specific library dependencies, it is best practice to pin both the version and the variant explicitly (e.g., redis:8.8.0-alpine3.23) [2][5]. For maximum reliability, some production environments may even pin to the specific image digest (e.g., redis@sha256:...) to guarantee that the exact same byte-for-byte image is used across all deployments [1][4].

Citations:


Pin Redis image to a specific version instead of redis:latest.
redis:latest is a mutable moving tag on Docker Hub, so reinstalls can pull a different Redis build over time. Pin to a fixed version tag for production (ideally a specific patch/variant, or even an image digest) rather than latest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/modules/software/module_redis.sh` at line 12, The Docker image for
Redis is pinned to the mutable tag in the setting
["module_redis,dockerimage"]="redis:latest"; replace that value with a fixed,
specific version tag (e.g. a minor/patch like redis:7.2.1) or an immutable image
digest to ensure reproducible installs and avoid pulling different builds over
time. Update the value for the ["module_redis,dockerimage"] entry and, if
possible, include the exact patch or digest you want the deployment to use.

["module_redis,dockerimage"]="redis:alpine"
["module_redis,dockerimage"]="redis:latest"
["module_redis,dockername"]="redis"
["module_redis,maxmemory"]="64gb"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Default maxmemory is dangerously high for many hosts.

Line 14 sets maxmemory to 64gb. On hosts with much less RAM, Redis won’t evict until approaching that threshold, so the kernel can OOM-kill workloads first. This is especially risky because dependent installers (for example NetBox/Immich paths) auto-install Redis when missing.

Suggested change
-	["module_redis,maxmemory"]="64gb"
+	["module_redis,maxmemory"]="1gb"

Also applies to: 68-68

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/modules/software/module_redis.sh` at line 14, The default Redis
maxmemory value is set too high ("module_redis,maxmemory" => "64gb"); change it
to a safe conservative default (e.g., "1gb") or make it unset so admins supply
it, and update the other occurrence of the same key as well; locate the setting
identified by "module_redis,maxmemory" in module_redis.sh and replace "64gb"
with the new safer default (or remove the default) so small hosts won't be
driven to OOM by Redis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

08 Milestone: Third quarter release Documentation Documentation changes or additions size/medium PR with more then 50 and less then 250 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant