Skip to content

feat(distributed-cache): Add distributed cache support - #310

Open
avasenin wants to merge 9 commits into
PostHog:mainfrom
avasenin:feat/flag-definition-cache-provider
Open

feat(distributed-cache): Add distributed cache support#310
avasenin wants to merge 9 commits into
PostHog:mainfrom
avasenin:feat/flag-definition-cache-provider

Conversation

@avasenin

@avasenin avasenin commented Sep 3, 2026

Copy link
Copy Markdown

💡 Motivation and Context

Our tech stack is ruby/go and we would like to use https://posthog.com/docs/feature-flags/local-evaluation/distributed-environments

This PR adds support of this feature for go sdk.

💚 How did you test it?

Run tests full suite, please i used example to test out that its working as expected, fetch flags definitions and re-use it and next fetch if needed

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.
  • I run examples manually with keys and make sure it works as it should

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Used existing 3 PRs in ruby/js/python as reference
  • Decided to not bring Redis dependency and use file cache in examples to not increase footprint

@avasenin
avasenin requested a review from a team as a code owner September 3, 2026 11:21
@marandaneto
marandaneto requested a review from a team September 3, 2026 11:40

@dustinbyrne dustinbyrne 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.

hey @avasenin, thanks for putting this together! i've used an agent to compare this to other implementations and have some findings below. i'm around if you want any of these taken off your plate. otherwise i'll leave it to you!

Comment thread flag_definition_cache.go
Comment thread flag_definition_cache.go Outdated
Comment thread featureflags.go Outdated
Comment thread featureflags.go Outdated
Comment thread config.go
Comment thread examples/flag_definition_cache.go Outdated
avasenin and others added 8 commits September 5, 2026 15:08
Share the server-controlled minimal_flag_called_events gate through the
cached payload so that followers emit the same $feature_flag_called events
as the instance that fetched the definitions.

Validate provider data at the cache boundary: definitions without a flags
key, and multivariate variants without a rollout percentage, are logged and
treated as a cache miss instead of erasing the definitions already loaded or
panicking during preprocessing.

Order and bound provider cleanup. shutdownPoller now waits for the polling
loop to return before calling Shutdown, and uses the client context rather
than its own 30 second budget, so a short CloseWithContext deadline is
honoured.

Force keyed literals on FlagDefinitionCacheData, add the changeset, and fix
the instance count in the example.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… guide

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ments

Co-Authored-By: Claude <noreply@anthropic.com>
…annels

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@avasenin

avasenin commented Sep 5, 2026

Copy link
Copy Markdown
Author

hey @dustinbyrne . i think i finished with all comments. let me know what do u think

@avasenin
avasenin requested a review from dustinbyrne September 5, 2026 16:15
@dustinbyrne
dustinbyrne requested a review from a team September 7, 2026 02:58
@avasenin avasenin changed the title Add distributed cache support feat(distributed-cache): Add distributed cache support Sep 7, 2026

@dustinbyrne dustinbyrne 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.

thanks @avasenin! this is looking great. a couple of additional items as i'm looking through this. again, happy to take anything off your plate if you wish.

lastly - and i missed this on this first look - we require commit signing in order to merge to main. can you enable that and rebase/force push to sign your commits?

Comment thread featureflags.go
poller.Logger.Warnf("[FEATURE FLAGS] Polling loop did not stop before the shutdown deadline: %s", ctx.Err())
}

poller.shutdownCacheProvider(ctx)

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.

Keep provider cleanup ordered after active polling work

When ctx.Done() wins the select above, this still calls provider Shutdown even though the poller has not finished. Provider operations receive context.Background(), so client cancellation cannot stop them.

A public-client regression with a context-aware provider and a 20 ms CloseWithContext deadline reproduces Shutdown overlapping an active ShouldFetchFlagDefinitions, followed by GetFlagDefinitions after cleanup returns. Depending on the provider, that can use a closed connection or acquire a lock after cleanup already attempted to release it.

Consider keeping the caller's wait bounded while cancelling provider work, preventing further provider calls, and ensuring cleanup runs only after active polling finishes. Add a regression combining an active provider operation with an expiring shutdown deadline; the existing tests cover ordering and timeout separately.

Comment thread flag_definition_cache.go
type FlagDefinitionCacheData struct {
_ struct{}

Flags []FeatureFlag `json:"flags"`

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.

Preserve the complete shared-cache payload rather than the evaluator model

Using []FeatureFlag makes the cache payload a projection of Go's evaluator model. That type omits fields such as flag id and version: I reproduced both disappearing when endpoint-shaped JSON is decoded into FlagDefinitionCacheData and marshalled again, as the file-cache example does. A different SDK or SDK version reading the same cache may need fields this evaluator does not use.

Consider using json.RawMessage at the provider boundary and retaining the original endpoint-shaped definition payload for publication, with typed decoding kept inside the SDK for validation and evaluation. Serializing the existing typed model into RawMessage would still lose those fields. Add round-trip coverage preserving id, version, and unknown metadata, including nested fields.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants