feat(flags): add feature_flags_async_load option to keep definition fetches off the calling thread#221
Merged
Conversation
5 tasks
…etches off the calling thread
Written on the poller thread (always, with async load; previously only after a failed initial load) and read from application threads, so it needs a visibility guarantee.
flivni
force-pushed
the
async-flag-definitions-load
branch
from
July 17, 2026 20:42
292561d to
4a9f50b
Compare
# Conflicts: # lib/posthog/feature_flags.rb
turnipdabeets
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
PostHog's Server-side Local Evaluation feature improves
performance by evaluating flags in-process, with definitions refreshed by a background polling thread.
The problem is ... definitions are not always requested in a background thread. This could result in unexpected request-latency spikes which on a busy server could cascade into an outage.
Two issues
get_feature_flagreverts to synchronous behavior when flags have not yet successfully loaded. If, for example, PostHog is down on boot, every call toget_feature_flagwill be synchronous until a fetch succeeds; request times might go from ms to seconds. Note: A 402 quota response clears cached definitions and thereby also re-arms inline fetching.PostHog::Client performs a blocking, synchronous fetch on initialization. In the common case, the block is perhaps 1-2s. But in the case where PostHog is down or there is network connectivity issue, this can be a 10s timeout. In clustered Puma with preload_app!, this can be paid twice serially (master boot, then on_worker_boot). Additionally, PR fix(flags): Retry flag requests on transient network errors #198 adds a retry, also doubling. Many platforms (e.g. Heroku) have boot time constraints. It's possible that a PostHog outage or network connectivity issue could cause apps that have strict boot timeouts to be unable to (re)start.
A solution
This PR aims to address both these issues by adding an initialization parameter
feature_flags_async_load, while, by default, maintaining the current, sometimes synchronous, behavior.When enabled, definitions are fetched only on the existing poller thread: its first tick fires immediately at boot (
run_now: true), and failures retry on the normal polling cadence. No new threads.Also introduced is a new public API method
feature_flags_loaded?which allows users to check if feature flags have been loaded (see code sample below).Fix: A 402 quota response now clears
flag_definitions_loaded_atalong with the definitions it discards, sofeature_flags_loaded?andevaluate_flags(...).flag_definitions_loaded_atno longer report definitions as loaded after a quota limit unloads them. This is a minor bugfix, but also a subtle observable change to an existing public field.Fix:
flag_definitions_loaded_atwas a plain ivar written by the poller thread (previously only when the initial synchronous load failed; with async load, always), so readers on other threads had no visibility guarantee on JRuby/TruffleRuby. It's now aConcurrent::AtomicReference, which also guarantees that whenfeature_flags_loaded?returns true, the definitions written before it are visible.When not to prefer the new fully async behavior
The new, fully asynchronous behavior might not always be preferred. One negative is that with the new
feature_flags_async_loadset totrue, requests following quickly after boot might, even in the healthy case, see nil values forget_feature_flagas the definitions have not yet loaded. But, IMHO, the new behavior is, in most cases, the desirable behavior.Users who prefer the default synchronous boot, but don't want evaluation calls to fetch synchronously when initial load (or subsequent 402 reload) failed, can use:
💚 How did you test it?
feature_flags_async_load_spec.rb, 7 examples): constructor returns immediately and definitions load asynchronously; sync default unchanged when the option is off;evaluate_flagsreturns nil fast and starts zero additional fetches while the initial load is in flight; recovery happens on the polling cadence after a failed (500) initial load;reload_feature_flagsstill fetches synchronously;feature_flags_loaded?is false without asecret_keyand goes false again when a 402 discards the definitions. Async waits use the suite's existingeventuallyhelper.rubocopclean.rake public_api:checkpasses with the snapshot regenerated for the one new public method.Client.new📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (Claude Fable 5 assisted)
Design decisions were human, notably relying on the existing poller timer rather than introducing new threads or concurrency primitives. Product code is ~90% human-written with AI review; tests ~90% AI-written, human-led; documentation AI-outlined, human-written, then AI-copyedited.
An AI multi-agent code review of the branch produced findings that were human-adjudicated: some were fixed here (a cross-thread visibility fix, spec cleanup), several were determined to be pre-existing issues and queued as follow-up PRs, and the rest rejected with rationale.
The human understands all the code in this PR.