graphql-pro: RedisStorage clean_inactive_subs leaves dead subscription IDs in mixed fingerprint sets under TTL expiry

Start with the clean_inactive_subs Lua script in GraphQL::Pro::Subscriptions::RedisStorage and compare its fingerprint cleanup with the legacy filter_active_subscriptions path. Use the mixed live/dead fingerprint reproduction with a short stale_ttl_s, then verify that the expired bare subscription ID is removed from the fingerprintsubscriptions set while the live ID remains.

This is a graphql-pro bug (closed-source), but Pro has no public issue tracker, so I'm filing here to reach you — happy to move it to support@graphql.pro if you'd prefer. We're a Pro license holder.

Gem / location

• graphql-pro 1.30.0
• GraphQL::Pro::Subscriptions::RedisStorage, in the clean_inactive_subs Lua script used by each_fingerprint_and_subscription_ids.

Summary

When a subscription's query record is removed by Redis TTL (stale_ttl_s) rather than by an explicit delete_subscription, the cleanup that runs while iterating a topic's fingerprints fails to remove that subscription id from its fingerprintsubscriptions:<fingerprint> set — unless every member of that set is dead. Any fingerprint set that still has at least one live member accumulates dead ids indefinitely, so fingerprintsubscriptions:* grows without bound.

Root cause

The fingerprintsubscriptions:<fingerprint> set is populated with the bare subscription id (via sadd(fingerprint_subscriptions_key(f), [sub_id]) in the write path). But in clean_inactive_subs, the SREM against that set is issued with the prefixed query key (<prefix>query:<id>) — the same variable used for the preceding DEL — instead of the bare id. Since that value is never a member of the set, the SREM is a silent no-op and the dead id is left behind.

The leak is masked for fully-dead sets only because run_fingerprint_batch separately DELs the whole set when no active ids remain. Mixed (live + dead) sets are where it leaks.

Strong signal that this is the bug and not intended: the legacy path (filter_active_subscriptions) does it correctly — it SREMs the bare inactive_subscription_ids. Only the fingerprint path uses the prefixed key.

Suggested fix

In clean_inactive_subs, SREM the bare subscription id from fingerprintsubscriptions:<fingerprint> (the bare ids are already available 1:1 alongside the prefixed query keys), matching what the legacy filter_active_subscriptions path does.

Reproduction

1. Configure RedisStorage with a short stale_ttl_s.
2. Create two subscriptions whose queries hash to the same fingerprint (same query + variables), so both ids land in one fingerprintsubscriptions:<fp> set.
3. Let one subscription's query record expire via TTL while the other stays live.
4. Trigger the topic (runs each_fingerprint_and_subscription_ids).
5. Observe: the expired id is still a member of fingerprintsubscriptions:<fp> (SMEMBERS lists it) and is never reaped.

Impact (our production)

Redis backing GraphQL subscriptions roughly doubled in memory over a few months. ~96% of keys were gql:sub:*, and a sampled ~99% of fingerprintsubscriptions members pointed at query records that had already expired. The affected pattern is recurring per-user fingerprints (e.g. mobile clients that reconnect with a fresh subscription id but an identical query+variables fingerprint): the set always keeps one live member, so the bug applies and dead ids pile up.

Thanks for all your work on graphql-ruby/pro!