fix: delta caching by environment#1422
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| self.events.sort_by_key(|event| event.get_event_id()); | ||
| if self.events.len() > self.max_length { | ||
| let to_remove = self.events.len() - self.max_length; | ||
| self.events.drain(0..to_remove); | ||
| } |
There was a problem hiding this comment.
Keep a bounded, deterministic window of events.
Events may arrive out of order, so we sort by event_id before trimming.
This ensures we always drop the logically oldest events (lowest event_id), not just the first inserted ones, and keeps hydration state deterministic.
There was a problem hiding this comment.
Pull request overview
This PR fixes delta-cache behavior when multiple tokens/projects share the same environment cache key by merging hydration results per-project (instead of replacing the whole environment state), and adds tests to ensure deterministic delta ordering and hydration isolation.
Changes:
- Update delta refresh handling to merge hydration by project into the shared environment cache and rebuild the engine from the merged feature set on hydration.
- Extend delta cache manager/cache to support project-scoped hydration merges and deterministic event ordering.
- Add tests covering deterministic delta ordering and ensuring one project’s hydration doesn’t remove other projects’ features.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
crates/oss/unleash-edge-feature-refresh/src/delta_refresh.rs |
Extract hydration events, merge hydrated state per project into the shared environment cache, and rebuild engine state from the merged cache on hydration; adds regression test for project hydration isolation. |
crates/enterprise/unleash-edge-delta/src/cache_manager.rs |
Adds merge_hydration_cache API to merge hydration into an existing environment cache (or insert if missing). |
crates/enterprise/unleash-edge-delta/src/cache.rs |
Makes event ordering deterministic and introduces project-scoped hydration merge helpers for the delta cache. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn add_events(&mut self, events: &[DeltaEvent]) { | ||
| for event in events.iter() { | ||
| self.events.push(event.clone()); | ||
| self.update_hydration_event(event); | ||
|
|
||
| if self.events.len() > self.max_length { | ||
| self.events.remove(0); | ||
| } | ||
| } | ||
| self.events.sort_by_key(|event| event.get_event_id()); | ||
| if self.events.len() > self.max_length { |
There was a problem hiding this comment.
add_events() sorts events by event_id, which suggests callers may provide events out of order. However update_hydration_event() currently sets self.hydration_event.event_id = event.get_event_id() for every processed event, so if the input slice is out of order the stored hydration event_id can move backwards (e.g. processing 12 then 11 leaves event_id as 11). This can break revision comparisons in filter_deltas() (and any other logic using hydration_event.event_id). Consider tracking the max event id instead (e.g. event_id = max(event_id, incoming_id)), or recompute hydration_event.event_id from the max of self.events after sorting, and add an assertion/test that hydration_event.event_id stays monotonic.
| &mut self, | ||
| projects: &[String], | ||
| hydration_event: DeltaHydrationEvent, | ||
| ) { |
There was a problem hiding this comment.
merge_hydration_for_projects() overwrites self.hydration_event.event_id with the incoming hydration’s event_id. If hydrations can arrive out of order (e.g. concurrent refresh/SSE tasks across tokens), this can make the cached revision go backwards and confuse revision-based delta filtering. Consider guarding against regressions (e.g. keep the max event_id, or ignore/short-circuit older hydration events).
| ) { | |
| ) { | |
| // Guard against regressing the cached revision in case hydrations arrive out of order. | |
| if hydration_event.event_id <= self.hydration_event.event_id { | |
| return; | |
| } |
| .iter() | ||
| .filter(|toggle| { | ||
| let project = toggle.project.clone().unwrap_or_else(|| "default".into()); | ||
| !projects_to_update.contains(&project) |
There was a problem hiding this comment.
This is called on every feature, perhaps can be HashSet.
We fixed this problem by preventing edge from starting up with two tokens in the same environment when streaming is enabled |
About the changes
Fixes a streaming+delta cache bug where multiple tokens sharing the same environment could overwrite each other’s state (last token wins), causing one token to return empty features.
Problem
When two tokens target the same environment but different projects (for example project-1/development and project-2/development), hydration events were effectively treated as environment-wide replacements. This could remove flags belonging to other tokens in the same environment bucket.
Root Cause
Delta and feature hydration updates were keyed by environment and applied in a way that did not preserve project isolation during hydration updates.
What changed
*).Tests run