graphql-pro: RedisStorage clean_inactive_subs leaves dead subscription IDs in mixed fingerprint sets under TTL expiry
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 30/100
調査の方向性
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.
索引モデルが issue の本文から書いたものです。
説明
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 [email protected] if you'd prefer. We're a Pro license holder.
Gem / location
graphql-pro1.30.0GraphQL::Pro::Subscriptions::RedisStorage, in theclean_inactive_subsLua script used byeach_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
- Configure
RedisStoragewith a shortstale_ttl_s. - Create two subscriptions whose queries hash to the same fingerprint (same query + variables), so both ids land in one
fingerprintsubscriptions:<fp>set. - Let one subscription's query record expire via TTL while the other stays live.
- Trigger the topic (runs
each_fingerprint_and_subscription_ids). - Observe: the expired id is still a member of
fingerprintsubscriptions:<fp>(SMEMBERSlists 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!
- 主要言語
- Ruby
- スター
- 5.4k
- フォーク
- 1.4k
- 平均マージ
- 1日 8時間
- マージ済み PR（30日）
- 74
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
rmosolgo/graphql-ruby のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
rmosolgo/graphql-ruby#5843 ·
メンテナーはふだん 1 日以内に返信
-
難易度 3/5 1〜2日 初心者へのやさしさ 55/100
rmosolgo/graphql-ruby#5763 ·
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
rmosolgo/graphql-ruby#5761 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 52/100
rmosolgo/graphql-ruby#5707 · コメント 10 件 ·
メンテナーはふだん 1 日以内に返信
-
Cache complexity analyzer in-memory/redis対応中かも このイシューにリンクされたプルリクエストがオープン中、またはマージ済みです。 オープン
難易度 5/5 1週間以上 初心者へのやさしさ 35/100
rmosolgo/graphql-ruby#5632 · コメント 4 件 ·
メンテナーはふだん 1 日以内に返信
rmosolgo/graphql-ruby の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
メンテナーはふだん 1 日以内に返信
-
account: profile
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
メンテナーはふだん 5 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
メンテナーはふだん 1 日以内に返信
-
bug good first issue help wanted
難易度 2/5 1〜3時間 初心者へのやさしさ 90/100
ysb3996-blip/oops#24 ·
メンテナーはふだん 1 日以内に返信