Skip to content

Commit ea17419

Browse files
junwoo-choi-rapportlabsntkathole
authored andcommitted
fix: Prevent mutation of cached feature resolution results
Copy grouped feature lists before passing them to online stores so Redis timestamp fields do not accumulate in the shared resolution cache. Signed-off-by: rapportlabs <[email protected]>
1 parent d1c709d commit ea17419

2 files changed

Lines changed: 38 additions & 0 deletions

File tree

‎sdk/python/feast/utils.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1568,6 +1568,13 @@ def _prepare_entities_to_read_from_online_store(
15681568
entityless_case,
15691569
) = _get_cached_request_context(registry, project, features, full_feature_names)
15701570

1571+
# Online stores may append internal fields to the requested feature lists.
1572+
# Keep those request-local so the cached resolution result remains unchanged.
1573+
grouped_refs = [
1574+
(feature_view, list(requested_features))
1575+
for feature_view, requested_features in grouped_refs
1576+
]
1577+
15711578
# Mutable copy — downstream code adds join keys to this set.
15721579
requested_result_row_names = set(requested_result_row_names_frozen)
15731580

‎sdk/python/tests/unit/test_feature_resolution_cache.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import pytest
77

88
import feast.utils as utils
9+
from feast.protos.feast.types.Value_pb2 import Value as ValueProto
910

1011

1112
@pytest.fixture(autouse=True)
@@ -140,6 +141,36 @@ def test_returned_result_row_names_is_mutable_copy(self, mock_ctx):
140141
assert isinstance(result_row_names_1, frozenset)
141142
assert result_row_names_1 == result_row_names_2
142143

144+
@patch("feast.utils._get_online_request_context")
145+
def test_grouped_refs_are_copied_per_request(self, mock_ctx):
146+
"""Downstream mutations must not leak into cached grouped refs."""
147+
mock_ctx.return_value = _make_context()
148+
registry = _make_registry(datetime.now(tz=timezone.utc))
149+
entity_values = {"user_id": [ValueProto(int64_val=1)]}
150+
151+
first_result = utils._prepare_entities_to_read_from_online_store(
152+
registry,
153+
"proj",
154+
["fv:feat1"],
155+
entity_values,
156+
native_entity_values=False,
157+
)
158+
first_requested_features = first_result[1][0][1]
159+
first_requested_features.append("_ts:fv")
160+
161+
second_result = utils._prepare_entities_to_read_from_online_store(
162+
registry,
163+
"proj",
164+
["fv:feat1"],
165+
entity_values,
166+
native_entity_values=False,
167+
)
168+
second_requested_features = second_result[1][0][1]
169+
assert first_requested_features == ["feat1", "_ts:fv"]
170+
assert second_requested_features == ["feat1"]
171+
assert second_requested_features is not first_requested_features
172+
mock_ctx.assert_called_once()
173+
143174
@patch("feast.utils._get_online_request_context")
144175
def test_feature_service_cached_separately(self, mock_ctx):
145176
"""FeatureService objects should be cached by name."""

0 commit comments

Comments
 (0)