docs: replace outdated State Resync Events section - #2026
Conversation
flagd no longer emits state resync events. Since flag storage moved to keying flags by flag set ID and key, `SetState()` no longer returns a resync signal, and `Store.Update` deletes only the rows belonging to the source that dropped the flag. Nothing re-requests the full definition from the remaining sources or rebuilds the merged state. Because only the highest priority definition of a given flag set ID and key is stored, removing that definition leaves no flag behind at all, so the documented fallback to a lower priority source no longer happens. Document the current behaviour and the resulting guideline that a flag must be supplied by exactly one source, and drop the two diagrams that illustrated the resync flow. Closes open-feature#1809 Signed-off-by: Vaibhav Srivastava <vaibhavsri1712@gmail.com>
dosubot
Bot
added
the
size:S
✅ Deploy Preview for polite-licorice-3db33c ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe sync documentation replaces the state resynchronization section with guidance on duplicate flags across sources, deletion behavior, evaluation results, and unique flag set ID/key requirements. ChangesSync source documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This is a localized documentation update with no runtime or product behavior changes, and no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
1 participant
Closes #1809.
The State Resync Events section documents a fallback that flagd no longer performs. This replaces it with the current behaviour and states the resulting guideline.
What I verified in the code first
Rather than take the issue at face value, I traced the current behaviour:
SetState()no longer returns a resync signal, removed in feat!: enable parsing of array flag configurations for flagd #1797 (its description confirms theIEvaluatorcontract "no longer returns detailed event information or a resync boolean").Store.Updatehandles a dropped flag bytxn.DeleteAll(flagsTable, flagSetIdKeySourceCompoundIndex, ...), scoped to the source that dropped it. Nothing re-requests definitions from the other sources.ISync.ReSync()still exists, but it is a sync source operation for fetching a full configuration, and its only non-test callers are inflagd-proxys subscription manager. It is not the merged-state resync the docs describe, so I left it alone.The important detail is why the fallback is gone rather than merely dormant. The primary index is unique on flag set ID + key, and flags without a
flagSetIdshare one process-widenilFlagSetId. So in the docs example,source-Aandsource-Bsfooare never stored at all, only the winner fromsource-C. Whensource-Cdropsfoo, there is nothing left to fall back to, and the flag disappears entirely. That is exactly theFLAG_NOT_FOUNDcase the old text promised resync would prevent.Changes
flagSetIdshare an implicit flag set, so plain duplicate keys collide the same way.flag-merge-3.svgandflag-merge-4.svg. They only illustrated the resync flow and were referenced solely by this section. Happy to restore them if you would rather keep the assets.markdownlint-cli2passes on the file, including the reposmax-one-sentence-per-linerule.One thing I noticed but left alone
core/pkg/evaluator/json_test.go:1039still declaresexpectedResync boolin theTestState_Evaluatortable. It is dead, with no other reference anywhere in the repo, and looks like a leftover from #1797. I kept this PR docs-only to match the issue, but happy to remove it here or in a follow-up.