Skip to content

Commit d8e7a99

Browse files
aniketpaluaniketpalu
committed
fix: Address #6649 review — run_async, race reserve, version threading
- Revert FeatureView.state from __eq__ - Add run_async for remote sync vs async HTTP - Reserve MATERIALIZING before 202; idempotent store transitions - Thread version through authorize and all materialize server paths - Narrow silent excepts; document version on request models Signed-off-by: Aniket Paluskar <apaluska@redhat.com>
1 parent dc6f965 commit d8e7a99

3 files changed

Lines changed: 104 additions & 33 deletions

File tree

‎sdk/python/feast/feature_server.py‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
from fastapi.logger import logger
4343
from fastapi.responses import JSONResponse
4444
from fastapi.staticfiles import StaticFiles
45-
from pydantic import BaseModel, field_validator
45+
from pydantic import BaseModel, Field, field_validator
4646

4747
import feast
4848
from feast import metrics as feast_metrics
@@ -96,14 +96,26 @@ class MaterializeRequest(BaseModel):
9696
feature_views: Optional[List[str]] = None
9797
disable_event_timestamp: bool = False
9898
full_feature_names: bool = False
99-
version: Optional[str] = None
99+
version: Optional[str] = Field(
100+
None,
101+
description=(
102+
"Optional version to materialize (e.g. 'v2'). Requires feature_views "
103+
"with exactly one entry and registry.enable_online_feature_view_versioning."
104+
),
105+
)
100106

101107

102108
class MaterializeIncrementalRequest(BaseModel):
103109
end_ts: str
104110
feature_views: Optional[List[str]] = None
105111
full_feature_names: bool = False
106-
version: Optional[str] = None
112+
version: Optional[str] = Field(
113+
None,
114+
description=(
115+
"Optional version to materialize (e.g. 'v2'). Requires feature_views "
116+
"with exactly one entry and registry.enable_online_feature_view_versioning."
117+
),
118+
)
107119

108120

109121
class GetOnlineFeaturesRequest(BaseModel):
@@ -340,14 +352,16 @@ async def load_static_artifacts(app: FastAPI, store):
340352
def _authorize_materialize_views(
341353
store: "feast.FeatureStore",
342354
feature_view_names: Optional[List[str]],
355+
version: Optional[str] = None,
343356
) -> List[str]:
344357
"""Resolve + authorize feature views for materialization.
345358
346359
Returns the resolved list of FV names (all eligible FVs when
347360
feature_view_names is None).
348361
"""
362+
parsed_version = store._validate_materialize_version(version, feature_view_names)
349363
feature_views_to_materialize = store._get_feature_views_to_materialize(
350-
feature_view_names
364+
feature_view_names, version=parsed_version
351365
)
352366
for fv in feature_views_to_materialize:
353367
assert_permissions(
@@ -370,8 +384,12 @@ def _check_already_materializing(
370384
)
371385
if getattr(fv, "state", None) == FeatureViewState.MATERIALIZING:
372386
conflicting.append(fv_name)
373-
except Exception:
387+
except (FeatureViewNotFoundException, KeyError):
374388
pass
389+
except Exception as e:
390+
logger.warning(
391+
f"Unexpected error checking MATERIALIZING state for {fv_name}: {e}"
392+
)
375393
if conflicting:
376394
return JSONResponse(
377395
status_code=409,
@@ -400,18 +418,17 @@ def _update_fv_state(
400418
)
401419
fv.state = state
402420
store.registry.apply_feature_view(fv, store.project)
403-
except Exception:
404-
logger.warning(f"Failed to set state={state} for {fv_name}")
421+
except (FeatureViewNotFoundException, KeyError):
422+
logger.warning(f"Feature view {fv_name} not found; skip state={state}")
423+
except Exception as e:
424+
logger.warning(f"Failed to set state={state} for {fv_name}: {e}")
405425

406426

407427
def _reset_stuck_materializing_to_generated(
408428
store: "feast.FeatureStore",
409429
fv_names: List[str],
410430
) -> None:
411-
"""Reset FVs currently in MATERIALIZING to GENERATED (force override).
412-
413-
Leaves other states untouched so store.materialize() can transition normally.
414-
"""
431+
"""Reset FVs currently in MATERIALIZING to GENERATED (force override)."""
415432
stuck: List[str] = []
416433
for fv_name in fv_names:
417434
try:
@@ -420,8 +437,10 @@ def _reset_stuck_materializing_to_generated(
420437
)
421438
if getattr(fv, "state", None) == FeatureViewState.MATERIALIZING:
422439
stuck.append(fv_name)
423-
except Exception:
440+
except (FeatureViewNotFoundException, KeyError):
424441
pass
442+
except Exception as e:
443+
logger.warning(f"Unexpected error while force-resetting {fv_name}: {e}")
425444
if stuck:
426445
_update_fv_state(store, stuck, FeatureViewState.GENERATED)
427446
logger.info(
@@ -923,7 +942,9 @@ async def materialize(
923942
force: bool = Query(False),
924943
):
925944
with feast_metrics.track_request_latency("/materialize"):
926-
fv_names = _authorize_materialize_views(store, request.feature_views)
945+
fv_names = _authorize_materialize_views(
946+
store, request.feature_views, version=request.version
947+
)
927948
start_date, end_date = _parse_materialize_timestamps(request)
928949

929950
if async_mode:
@@ -934,8 +955,10 @@ async def materialize(
934955
if conflict:
935956
return conflict
936957

937-
# State transitions (MATERIALIZING / AVAILABLE_ONLINE) are owned
938-
# by store.materialize(); server only accepts and runs in background.
958+
# Reserve MATERIALIZING before 202 so concurrent requests hit 409.
959+
# store.materialize() treats already-MATERIALIZING as a no-op.
960+
_update_fv_state(store, fv_names, FeatureViewState.MATERIALIZING)
961+
939962
def _run_materialize():
940963
try:
941964
store.materialize(
@@ -965,9 +988,10 @@ def _run_materialize():
965988
store.materialize,
966989
start_date,
967990
end_date,
968-
request.feature_views,
991+
fv_names,
969992
disable_event_timestamp=request.disable_event_timestamp,
970993
full_feature_names=request.full_feature_names,
994+
version=request.version,
971995
)
972996

973997
@app.post("/materialize-incremental", dependencies=[Depends(inject_user_details)])
@@ -977,7 +1001,9 @@ async def materialize_incremental(
9771001
force: bool = Query(False),
9781002
):
9791003
with feast_metrics.track_request_latency("/materialize-incremental"):
980-
fv_names = _authorize_materialize_views(store, request.feature_views)
1004+
fv_names = _authorize_materialize_views(
1005+
store, request.feature_views, version=request.version
1006+
)
9811007
end_date = utils.make_tzaware(parser.parse(request.end_ts))
9821008

9831009
if async_mode:
@@ -988,13 +1014,15 @@ async def materialize_incremental(
9881014
if conflict:
9891015
return conflict
9901016

991-
# State transitions owned by store.materialize_incremental().
1017+
_update_fv_state(store, fv_names, FeatureViewState.MATERIALIZING)
1018+
9921019
def _run_materialize_incremental():
9931020
try:
9941021
store.materialize_incremental(
9951022
end_date,
9961023
fv_names,
9971024
full_feature_names=request.full_feature_names,
1025+
version=request.version,
9981026
)
9991027
except Exception as e:
10001028
logger.error(
@@ -1014,8 +1042,9 @@ def _run_materialize_incremental():
10141042
await run_in_threadpool(
10151043
store.materialize_incremental,
10161044
end_date,
1017-
request.feature_views,
1045+
fv_names,
10181046
full_feature_names=request.full_feature_names,
1047+
version=request.version,
10191048
)
10201049

10211050
@app.exception_handler(Exception)

‎sdk/python/feast/feature_store.py‎

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -509,8 +509,15 @@ def _transition_fv_to_materializing(
509509
Transition a feature view to MATERIALIZING state.
510510
511511
Rolls back all already-transitioned FVs if this one can't transition.
512+
Already MATERIALIZING is a no-op (async server may have reserved the state
513+
before returning 202); rollback target is GENERATED in that case.
512514
"""
513-
previous_states[feature_view.name] = getattr(feature_view, "state", None)
515+
current = getattr(feature_view, "state", None)
516+
if current == FeatureViewState.MATERIALIZING:
517+
previous_states[feature_view.name] = FeatureViewState.GENERATED
518+
return
519+
520+
previous_states[feature_view.name] = current
514521
if (
515522
hasattr(feature_view, "state")
516523
and feature_view.state != FeatureViewState.STATE_UNSPECIFIED
@@ -2501,13 +2508,32 @@ def _delegate_remote_materialize(
25012508
endpoint: str,
25022509
payload: Dict[str, Any],
25032510
force: bool = False,
2511+
run_async: bool = True,
25042512
) -> None:
2505-
"""Fire-and-forget POST to feature server with ?async=true."""
2506-
query_params = {"async": "true"}
2513+
"""POST materialize to the feature server.
2514+
2515+
When run_async=True (default), sends ?async=true and returns after 202.
2516+
When run_async=False, omits async and blocks until the server finishes
2517+
synchronous materialization (HTTP response).
2518+
force=True is only valid with run_async=True (server force applies to async).
2519+
"""
2520+
if force and not run_async:
2521+
raise ValueError(
2522+
"force=True requires run_async=True. "
2523+
"force only overrides stuck MATERIALIZING on the async path."
2524+
)
2525+
2526+
query_params: Dict[str, str] = {}
2527+
if run_async:
2528+
query_params["async"] = "true"
25072529
if force:
25082530
query_params["force"] = "true"
2509-
result = self._post_to_feature_server(endpoint, payload, query_params)
2510-
_logger.info("Remote materialization accepted (%s): %s", endpoint, result)
2531+
2532+
result = self._post_to_feature_server(endpoint, payload, query_params or None)
2533+
if run_async:
2534+
_logger.info("Remote materialization accepted (%s): %s", endpoint, result)
2535+
else:
2536+
_logger.info("Remote materialization completed (%s): %s", endpoint, result)
25112537

25122538
def materialize_incremental(
25132539
self,
@@ -2516,6 +2542,7 @@ def materialize_incremental(
25162542
full_feature_names: bool = False,
25172543
version: Optional[str] = None,
25182544
force: bool = False,
2545+
run_async: bool = True,
25192546
) -> None:
25202547
"""
25212548
Materialize incremental new data from the offline store into the online store.
@@ -2534,8 +2561,11 @@ def materialize_incremental(
25342561
feature view name.
25352562
version (str): Optional version to materialize (e.g., 'v2'). Requires feature_views
25362563
with exactly one entry and enable_online_feature_view_versioning to be enabled.
2537-
force (bool): When using remote topology, pass force=true to override stuck
2538-
MATERIALIZING state on the feature server. Ignored for local topology.
2564+
force (bool): When using remote topology with run_async=True, pass force=true to
2565+
override stuck MATERIALIZING state on the feature server. Ignored for local topology.
2566+
run_async (bool): When using remote topology, if True (default) POST with ?async=true
2567+
and return after 202. If False, POST without async and block until the server
2568+
finishes sync materialization. Ignored for local topology.
25392569
25402570
Raises:
25412571
Exception: A feature view being materialized does not have a TTL set.
@@ -2560,7 +2590,10 @@ def materialize_incremental(
25602590
if version is not None:
25612591
payload["version"] = version
25622592
self._delegate_remote_materialize(
2563-
"/materialize-incremental", payload, force=force
2593+
"/materialize-incremental",
2594+
payload,
2595+
force=force,
2596+
run_async=run_async,
25642597
)
25652598
return
25662599

@@ -2659,7 +2692,9 @@ def tqdm_builder(length):
26592692
else:
26602693
for feature_view, start_date in regular_fvs_with_dates:
26612694
previous_state = getattr(feature_view, "state", None)
2662-
if (
2695+
if previous_state == FeatureViewState.MATERIALIZING:
2696+
previous_state = FeatureViewState.GENERATED
2697+
elif (
26632698
hasattr(feature_view, "state")
26642699
and feature_view.state != FeatureViewState.STATE_UNSPECIFIED
26652700
):
@@ -2751,6 +2786,7 @@ def materialize(
27512786
full_feature_names: bool = False,
27522787
version: Optional[str] = None,
27532788
force: bool = False,
2789+
run_async: bool = True,
27542790
) -> None:
27552791
"""
27562792
Materialize data from the offline store into the online store.
@@ -2769,8 +2805,11 @@ def materialize(
27692805
feature view name.
27702806
version (str): Optional version to materialize (e.g., 'v2'). Requires feature_views
27712807
with exactly one entry and enable_online_feature_view_versioning to be enabled.
2772-
force (bool): When using remote topology, pass force=true to override stuck
2773-
MATERIALIZING state on the feature server. Ignored for local topology.
2808+
force (bool): When using remote topology with run_async=True, pass force=true to
2809+
override stuck MATERIALIZING state on the feature server. Ignored for local topology.
2810+
run_async (bool): When using remote topology, if True (default) POST with ?async=true
2811+
and return after 202. If False, POST without async and block until the server
2812+
finishes sync materialization. Ignored for local topology.
27742813
27752814
Examples:
27762815
Materialize all features into the online store over the interval
@@ -2795,7 +2834,9 @@ def materialize(
27952834
}
27962835
if version is not None:
27972836
payload["version"] = version
2798-
self._delegate_remote_materialize("/materialize", payload, force=force)
2837+
self._delegate_remote_materialize(
2838+
"/materialize", payload, force=force, run_async=run_async
2839+
)
27992840
return
28002841

28012842
if utils.make_tzaware(start_date) > utils.make_tzaware(end_date):
@@ -2864,7 +2905,9 @@ def tqdm_builder(length):
28642905
else:
28652906
for feature_view, fv_start in regular_fvs_with_dates:
28662907
previous_state = getattr(feature_view, "state", None)
2867-
if (
2908+
if previous_state == FeatureViewState.MATERIALIZING:
2909+
previous_state = FeatureViewState.GENERATED
2910+
elif (
28682911
hasattr(feature_view, "state")
28692912
and feature_view.state != FeatureViewState.STATE_UNSPECIFIED
28702913
):

‎sdk/python/feast/feature_view.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,6 @@ def __eq__(self, other):
414414
or normalize_version_string(self.version)
415415
!= normalize_version_string(other.version)
416416
or self.org != other.org
417-
or self.state != other.state
418417
):
419418
return False
420419

0 commit comments

Comments
 (0)