feat: Add redis backend for Autopush by jrconlin · Pull Request #1039 · mozilla-services/autopush-rs

jrconlin · 2025-12-10T23:31:37Z

This PR is taken from #813, which adds a Redis backend data store. The
original patch was written by @p1gp1g.

Note that this patch was originally created for small, stand-alone
server uses, and has not been fully load tested.

@p1gp1g

This PR is taken from #813, which adds a Redis backend data store. The original patch was written by [@p1gp1g](https://github.com/p1gp1g). Note that this patch was originally created for small, stand-alone server uses, and has not been fully load tested. Closes PUSH-587

… feat/PUSH-587_redis

Copilot

Pull request overview

This PR adds Redis as an alternative backend storage option for Autopush, based on original work by @p1gp1g. The implementation provides a complete Redis-based data store for notification and routing information, intended for small, stand-alone server deployments.

Key Changes:

Implements a new Redis client (RedisClientImpl) with full DbClient trait support for user management, channel subscriptions, and message storage
Updates build system and feature flags to support both BigTable and Redis backends
Upgrades several dependencies (actix-web, sentry, uuid, ctor, mockall, hyper, prometheus-client) to newer versions

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 21 comments.

Show a summary per file

File	Description
autopush-common/src/db/redis/redis_client/mod.rs	Core Redis client implementation with connection management, key generation, and DbClient trait methods
autopush-common/src/db/redis/mod.rs	Redis settings configuration and deserialization
autopush-common/src/db/redis/redis_client/error.rs	Error handling conversions for serde_json errors
autopush-common/src/db/routing.rs	Adds Redis to StorageType enum and routing logic
autopush-common/src/db/models.rs	Adds feature gates to limit RangeKey to bigtable-only usage
autopush-common/src/db/mod.rs	Integrates Redis StorageType at the module level
autopush-common/src/notification.rs	Removes skip_serializing from timestamp field for Redis compatibility
autopush-common/src/db/error.rs	Adds RedisError variant to DbError enum
autopush-common/src/db/bigtable/bigtable_client/mod.rs	Updates import path for RangeKey
autopush-common/build.rs	Updates build check to accept either bigtable or redis features
autopush-common/Cargo.toml	Adds redis feature flag and updates prometheus-client version
autoendpoint/src/server.rs	Adds Redis client instantiation in server initialization
autoendpoint/Cargo.toml	Adds redis feature flag
autoconnect/autoconnect-settings/src/lib.rs	Adds Redis test settings and enables unsafe test for env vars
autoconnect/autoconnect-settings/src/app_state.rs	Adds Redis client initialization in app state
autoconnect/autoconnect-settings/Cargo.toml	Adds redis and unsafe feature flags
autoconnect/Cargo.toml	Adds redis and unsafe features to workspace
Cargo.toml	Updates dependencies and removes bigtable from default autopush_common features
Cargo.lock	Updates lock file with new dependency versions
Dockerfile	Adds BUILD_ARGS parameter for conditional feature builds
Makefile	Adds docker-dev-build target
.devcontainer/devcontainer.json	Adds cmake and INDOCKER environment variable

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

autopush-common/src/db/redis/redis_client/mod.rs

autopush-common/src/notification.rs

Cargo.toml

autopush-common/src/db/redis/redis_client/mod.rs

autopush-common/src/db/redis/mod.rs

autopush-common/src/db/redis/redis_client/mod.rs

* Only use `secs` since converting between secs and millis was causing potential errors

… feat/PUSH-587_redis

gruberb

Left a view comments. At least the creation of the connections can cause a race condition, so multiple connections could be created at once I believe.

autopush-common/src/db/redis/redis_client/mod.rs

autopush-common/src/db/redis/mod.rs

autopush-common/src/db/redis/redis_client/mod.rs

With the (Trixie+) container, it would be useful to be able to specify the emulator or test redis hosts instead of "localhost". This reuses a couple of existing environment variables `BIGTABLE_EMULATOR_HOST` and `REDIS_HOST`. Be sure to specify the IP and Port (e.g. `export BIGTABLE_EMULATOR_HOST=192.168.0.255:8080`)

gruberb

Left a view more comments! Sorry, didn't catch them earlier. It's a HUGE PR :)

autopush-common/src/db/redis/redis_client/mod.rs

gruberb · 2026-01-08T13:42:40Z

autopush-common/src/db/redis/redis_client/mod.rs

+    /// NOTE: There is some, very small, potential risk that a desktop client that can
+    /// somehow remain connected the duration of MAX_ROUTER_TTL, may be dropped as not being
+    /// "lively".
+    async fn update_user(&self, user: &mut User) -> DbResult<bool> {


Might be a millisecnd problem, but technically, if a User has a mobile and desktop app open, network issues etc, we can get duplicated HELLO messages at roughly the same time. So we could have two writes, and then we could send notification out to a stale connection.

Redis I believe doesn't have a form of lock, just a WATCH and UNWATCH. I don't know if we want to introduce this here (we need a loop, it's not a whole lot more code, maybe 5-10 lines). But we should leave a comment that this can happen? Might be one of these nasty bugs which is really hard to find later.

I'm not sure that this is a problem.
The desktop app and the mobile app should have two different UAIDs. What can happen is that a UAID could do a HELLO, then the connection is reset (and we don't get the websocket report), and the same UAID tries again. This is a VERY rare event (so much so that we stopped tracking it because after 5 years we never once got a hit).

autopush-common/src/db/redis/mod.rs

gruberb · 2026-01-08T13:45:37Z

autopush-common/src/notification.rs

    #[serde(rename = "channelID")]
    pub channel_id: Uuid,
    pub version: String,
+    pub timestamp: u64,


This was skipped before. Does this cause issues?

It shouldn't, but there's more to it.
Since the prior storage systems built the notifications artisanally, there was nothing that just dumped the notification as straight JSON. The original author decided that they would just re-use the Notification struct and make timestamp a real field, since it would be ignored by the UserAgent.
That's fine for small systems, but isn't a great idea, since there may be additional information that could differ from what we send out.
I'm going to create a generic StorableNotification that should be used in this case.

* Fix message purge to use full redis key path * Create semi-unique test id to ensure clean runs

* Introduce `StorableNotification` as a JSON formatter for `Notification` (since we may store different values than we transmit)

gruberb

👍 LGTM! One thing as a comment, not exactly sure how severe it is! We can also land a second PR with a patch after this.

gruberb · 2026-01-09T18:34:58Z

autopush-common/src/db/redis/redis_client/mod.rs

+        };
+        Ok(self
+            .conn
+            .get_or_try_init(|| async {


This makes me think: What if the connection breaks after a while, have we a way to recover?

Yeah, the advise is to use a connection pool for this: https://docs.rs/redis/latest/redis/#connection-pooling.

I'll add that later.

wrenix · 2026-01-14T09:21:30Z

I miss documentation for configuration #1058

jrconlin added 6 commits December 9, 2025 16:44

f nightly

a47fbbc

Merge branch 'master' of github.com:mozilla-services/autopush-rs into…

4ecf3fe

… feat/PUSH-587_redis

f add redis files

97f1a17

f revert to use bigtable as default (doh)

dc628e6

f remove dbg!

1ab5790

jrconlin marked this pull request as ready for review December 10, 2025 23:59

jrconlin requested a review from Copilot December 11, 2025 00:00

Copilot started reviewing on behalf of jrconlin December 11, 2025 00:00 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

jrconlin added 4 commits December 10, 2025 17:08

f first pass on review

7ad4956

f add'l r's

d346113

f functional cleanup

2b64b26

* Only use `secs` since converting between secs and millis was causing potential errors

Merge branch 'master' of github.com:mozilla-services/autopush-rs into…

a49cc86

… feat/PUSH-587_redis

jrconlin requested review from a team and gruberb December 18, 2025 22:50

gruberb requested changes Jan 7, 2026

View reviewed changes

jrconlin added 2 commits January 7, 2026 12:59

f fix notif ttl set

9e9ae56

jrconlin requested a review from gruberb January 7, 2026 22:23

gruberb reviewed Jan 8, 2026

View reviewed changes

jrconlin added 3 commits January 8, 2026 14:53

f r's

26b022b

* Fix message purge to use full redis key path * Create semi-unique test id to ensure clean runs

f r's

d41f90f

* Introduce `StorableNotification` as a JSON formatter for `Notification` (since we may store different values than we transmit)

f clippy

f08d9c1

jrconlin requested a review from gruberb January 9, 2026 16:13

gruberb approved these changes Jan 9, 2026

View reviewed changes

jrconlin merged commit 33398b4 into master Jan 9, 2026
3 checks passed

jrconlin deleted the feat/PUSH-587_redis branch January 9, 2026 18:42

p1gp1g mentioned this pull request Jan 11, 2026

Feat: Add redis backend #813

Closed

jrconlin mentioned this pull request Jan 29, 2026

chore: tag 1.78.0 #1061

Open

Conversation

jrconlin commented Dec 10, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gruberb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gruberb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gruberb Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jrconlin Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gruberb Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jrconlin Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

gruberb left a comment

Choose a reason for hiding this comment

Uh oh!

gruberb Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

jrconlin Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wrenix commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jrconlin commented Dec 10, 2025 •

edited by atlassian bot

Loading