Repository navigation
fix: Report real materialization status for Kubernetes async runs and in the UI - #7153
patelchaitany wants to merge 3 commits into
Conversation
The Jobs tab is built only from saved materialization intervals, and an interval is saved only after a run succeeds. A feature view that is being materialized right now looked the same as an idle one. The /compute_engines REST endpoint now returns each feature view's lifecycle state, and the Jobs tab adds a RUNNING row for views in the MATERIALIZING state. The date range shows a dash because the registry does not know the range until the run finishes. Signed-off-by: Chaitany Patel <[email protected]>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7153 +/- ##
==========================================
+ Coverage 52.32% 52.69% +0.37%
==========================================
Files 459 459
Lines 58095 58181 +86
Branches 8560 8578 +18
==========================================
+ Hits 30397 30659 +262
+ Misses 25614 25433 -181
- Partials 2084 2089 +5
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
With synchronous=false the Kubernetes compute engine returned a job that was still WAITING. Materialize then recorded the interval, set the feature view to AVAILABLE_ONLINE and emitted an OpenLineage COMPLETE event before any data was written, and pod failures were never reported. Now both modes wait for the Job to finish. synchronous only controls whether pods run in batches of job_batch_size or all in one Job. Also add the update/teardown_infra hooks the ComputeEngine base class requires, so the engine can be instantiated again. Each run creates and cleans up its own Job and ConfigMap, so both are no-ops. Signed-off-by: Chaitany Patel <[email protected]>
patelchaitany
changed the title
franciscojavierarceo
left a comment
franciscojavierarceo
left a comment
There was a problem hiding this comment.
the new default wait path depends on terminal Job conditions being recognized. that needs to work before we make every Kubernetes materialization wait.
| # feature view to AVAILABLE_ONLINE before any data is written. | ||
| total_pods = len(paths) | ||
| job = self._await_path_materialization( | ||
| paths, feature_view, 0, total_pods, total_pods |
There was a problem hiding this comment.
this now sends synchronous=False through a poller whose KubernetesMaterializationJob.status() only examines conditions[0]. on Kubernetes 1.31+, SuccessCriteriaMet can precede Complete, and FailureTarget can precede Failed (Job conditions). with those condition lists and no active pods, the status stays WAITING even after the Job finishes, so this new default path cannot finish recording the materialization and reporting its outcome. we need to recognize terminal conditions anywhere in the list and add success/failure regression cases using real KubernetesMaterializationJob status parsing before routing the default mode through this loop.
KubernetesMaterializationJob.status() only looked at conditions[0]. On Kubernetes 1.31+ a Job can report SuccessCriteriaMet before Complete, or FailureTarget before Failed. With no active pods the status then stayed WAITING forever, so the materialization wait loop never finished. Check every condition for Complete or Failed, and report RUNNING only while pods are active. Signed-off-by: Chaitany Patel <[email protected]>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
What this PR does / why we need it:
Materialization status was wrong in two places: the Kubernetes engine reported success before any data was written, and the UI could never show a run in progress. This PR fixes both. Supersedes #7154.
1. Kubernetes engine: wait for the Job when
synchronous: falseWith
synchronous: false, the Kubernetes compute engine created one Job for all pods and returned it right away, while the Job was stillWAITING.materialize()treats any returned job that isn'tERRORas done, so it:AVAILABLE_ONLINE,COMPLETEevent,all before a single row reached the online store. If the pods later failed, Feast never found out. The UI and lineage still showed the run as successful.
Now
synchronous: falsewaits for the Job too, using the same_await_path_materializationloop the synchronous path already uses.synchronousnow only decides how pods are grouped:true: run the pods in Jobs ofjob_batch_size, one Job after another.false: run all pods in a single Job.Either way, the feature view is marked materialized only after the Job succeeds. ConfigMap cleanup and pod-log printing on failure now run in both modes.
Because every run now goes through this wait loop,
KubernetesMaterializationJob.status()has to recognize when a Job has finished. It used to read onlyconditions[0]. On Kubernetes 1.31+,SuccessCriteriaMetcan come beforeComplete, andFailureTargetbeforeFailed(terminal Job conditions). With no active pods, the status stayedWAITINGforever. It now checks every condition forCompleteorFailed, and reportsRUNNINGonly while pods are active.This also adds the
updateandteardown_inframethods thatComputeEnginerequires. Without them,KubernetesComputeEngine(...)raisedTypeError: Can't instantiate abstract class. Each run creates and deletes its own Job and ConfigMap, so there is no long-lived infrastructure, and both methods do nothing. The Spark application engine does the same.Behavior change:
feast materializewithsynchronous: falsenow blocks until the Job finishes instead of returning right away.2. UI: show runs in progress in the Jobs tab
The Compute & Jobs → Jobs tab builds its rows only from saved materialization intervals. An interval is saved only after a run succeeds, so a feature view that was being materialized looked exactly like an idle one. Every row said
SUCCEEDED, and the Running counter was always0.The registry already knows when a run is in progress:
materialize()moves the feature view toMATERIALIZINGbefore it starts and moves it on when it ends. This PR passes that state through to the UI.GET /compute_engines(REST) now returns each feature view's lifecyclestate, such as"MATERIALIZING". It isnullwhen the state was never set.useLoadComputeEnginereads the newstatefield.RUNNINGrow for every view inMATERIALIZING, and the Running counter and filter count it. Range start and end show—, because the registry doesn't know the date range until the run finishes.Screenshots
Both screenshots use the same local Feast repo.
driver_hourly_statshas two finished runs and is in theMATERIALIZINGstate. Both builds run against the real REST API.Before: the in-progress run doesn't appear, and Running = 0.
After: a
RUNNINGrow appears fordriver_hourly_stats, and Running = 1.Which issue(s) this PR fixes:
N/A
Checks
git commit -s)Testing Strategy
Unit tests:
sdk/python/tests/unit/infra/compute_engines/test_k8s_engine.pybuilds a realKubernetesComputeEnginewithsynchronous=Falseand a realKubernetesMaterializationJob. Only the Kubernetes API is mocked. The Job goes from pending to running, then ends with one of two Kubernetes 1.31+ condition lists:[SuccessCriteriaMet, Complete], which must giveSUCCEEDED[FailureTarget, Failed], which must giveERRORThe test checks that one Job is created with all paths and that materialize waits for the final status. Both cases fail on master. The test is skipped when the
kubernetespackage isn't installed.sdk/python/tests/unit/api/test_compute_engines_api.pychecks that the endpoint returnsstate:MATERIALIZING, orNonewhen unset.ui/src/pages/compute-engines/Index.test.tschecks two cases. AMATERIALIZINGview gets aRUNNINGrow on top of its finished runs. AnAVAILABLE_ONLINEview with no intervals gets no rows.Manual test: the UI screenshots above. The Kubernetes change has not been tested against a real cluster.
Misc
Failed runs still don't appear in the Jobs tab. The registry doesn't store failures: on error, the view goes back to
GENERATEDand no interval is saved. That needs a separate change.