Skip to content

Commit a9219d9

Browse files
adarshsmntkathole
adarshsm
authored and
ntkathole
committed
fix: Report single-feature-view spark_application materialization success
`SparkApplicationComputeEngine.materialize()` deletes the SparkApplication CR in its `finally` immediately after `_build_per_fv_jobs()` returns. For a single feature view, `_build_per_fv_jobs()` short-circuited and returned the live polling job. `FeatureStore._submit_and_process_materialization_jobs()` then re-checks `job.status()`, which re-queries the just-deleted CR, gets a 404, and raises `SparkApplication feast-sa-<id> not found` — for a materialization that actually succeeded and already updated the registry. Automation calling `feast materialize` or `/materialize` sees a failure and may retry completed work. The bug is deterministic for exactly one feature view. Resolve the per-FV outcome from registry state for every task count, exactly as the multi-FV path already does: a materialized FV (`AVAILABLE_ONLINE`) becomes a `CompletedMaterializationJob`, which reports SUCCEEDED without any Kubernetes call and is therefore safe after cleanup. The now-unused `job` parameter is dropped. Genuine failures still surface as ERROR (the FV is not `AVAILABLE_ONLINE`), and the Spark driver's error detail is already logged by `_wait_for_completion`. Updated the single-task unit test, which previously asserted the buggy behavior (return the live job, skip the registry), to require resolution from registry state with no CR query, and added a single-failed-FV case. Fixes #6673 Signed-off-by: adarshsm <adarshmudugal@deborhn.shop>
1 parent 9affee5 commit a9219d9

2 files changed

Lines changed: 56 additions & 17 deletions

File tree

‎sdk/python/feast/infra/compute_engines/spark_application/compute.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ def materialize(
223223
)
224224
try:
225225
self._wait_for_completion(job)
226-
return self._build_per_fv_jobs(registry, tasks, job_id, job)
226+
return self._build_per_fv_jobs(registry, tasks, job_id)
227227
finally:
228228
self._cleanup(job_id)
229229

@@ -252,7 +252,6 @@ def _build_per_fv_jobs(
252252
registry: BaseRegistry,
253253
tasks: List[MaterializationTask],
254254
job_id: str,
255-
job: SparkApplicationMaterializationJob,
256255
) -> List[MaterializationJob]:
257256
"""Build one independent job object per FV from registry state.
258257
@@ -262,10 +261,15 @@ def _build_per_fv_jobs(
262261
263262
Each returned job is an independent object so that a failed
264263
SparkApplication does not pollute the status of succeeded FVs.
265-
"""
266-
if len(tasks) <= 1:
267-
return [job for _ in tasks]
268264
265+
The per-FV outcome is resolved from registry state — never from the live
266+
polling job — for every task count. ``materialize()`` deletes the
267+
SparkApplication CR in its ``finally`` immediately after this returns, and
268+
``FeatureStore`` then re-checks ``status()``; a job that re-queried the
269+
now-deleted CR would 404 and report a false failure for a materialization
270+
that actually succeeded (#6673). ``CompletedMaterializationJob`` reports
271+
SUCCEEDED without any Kubernetes call, so it is safe after cleanup.
272+
"""
269273
jobs: List[MaterializationJob] = []
270274
for task in tasks:
271275
fv = registry.get_feature_view(task.feature_view.name, task.project)

‎sdk/python/tests/unit/infra/compute_engines/test_spark_application.py‎

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,7 @@ def test_build_per_fv_jobs_all_succeeded():
313313
task2.feature_view.name = "fv_2"
314314
task2.project = "test"
315315

316-
parent_job = SparkApplicationMaterializationJob("job1", "default", MagicMock())
317-
jobs = engine._build_per_fv_jobs(mock_registry, [task1, task2], "job1", parent_job)
316+
jobs = engine._build_per_fv_jobs(mock_registry, [task1, task2], "job1")
318317

319318
assert len(jobs) == 2
320319
assert all(isinstance(j, CompletedMaterializationJob) for j in jobs)
@@ -343,10 +342,7 @@ def test_build_per_fv_jobs_partial_failure():
343342
task_fail.feature_view.name = "fv_fail"
344343
task_fail.project = "test"
345344

346-
parent_job = SparkApplicationMaterializationJob("job1", "default", MagicMock())
347-
jobs = engine._build_per_fv_jobs(
348-
mock_registry, [task_ok, task_fail], "job1", parent_job
349-
)
345+
jobs = engine._build_per_fv_jobs(mock_registry, [task_ok, task_fail], "job1")
350346

351347
assert len(jobs) == 2
352348
assert isinstance(jobs[0], CompletedMaterializationJob)
@@ -355,22 +351,61 @@ def test_build_per_fv_jobs_partial_failure():
355351
assert "fv_fail" in str(jobs[1].error())
356352

357353

358-
# ── Test 15: _build_per_fv_jobs — single task returns parent job directly ──
354+
# ── Test 15: _build_per_fv_jobs — single succeeded task resolves from registry ──
355+
359356

357+
def test_build_per_fv_jobs_single_task_succeeded():
358+
"""Regression for #6673.
360359
361-
def test_build_per_fv_jobs_single_task():
360+
A single successful FV must resolve to a CompletedMaterializationJob from
361+
registry state, not to the live polling job. materialize() deletes the CR
362+
right after this returns, so a live job would 404 on FeatureStore's follow-up
363+
status() check and report a false failure. CompletedMaterializationJob needs
364+
no Kubernetes call.
365+
"""
362366
engine = _make_engine()
363367
mock_registry = MagicMock()
368+
fv = MagicMock()
369+
fv.name = "fv_1"
370+
fv.state = FeatureViewState.AVAILABLE_ONLINE
371+
mock_registry.get_feature_view.return_value = fv
372+
373+
task = MagicMock()
374+
task.feature_view.name = "fv_1"
375+
task.project = "test"
376+
377+
jobs = engine._build_per_fv_jobs(mock_registry, [task], "job1")
378+
379+
assert len(jobs) == 1
380+
assert isinstance(jobs[0], CompletedMaterializationJob)
381+
assert jobs[0].status() == MaterializationJobStatus.SUCCEEDED
382+
mock_registry.get_feature_view.assert_called_once_with("fv_1", "test")
383+
# No live SparkApplication CR is queried (it is about to be deleted).
384+
engine.custom_api.get_namespaced_custom_object.assert_not_called()
385+
386+
387+
# ── Test 15b: _build_per_fv_jobs — single failed task reports error, no CR query ──
388+
389+
390+
def test_build_per_fv_jobs_single_task_failed():
391+
"""A single unmaterialized FV reports ERROR without querying the (deleted) CR."""
392+
engine = _make_engine()
393+
mock_registry = MagicMock()
394+
fv = MagicMock()
395+
fv.name = "fv_1"
396+
fv.state = FeatureViewState.MATERIALIZING
397+
mock_registry.get_feature_view.return_value = fv
398+
364399
task = MagicMock()
365400
task.feature_view.name = "fv_1"
366401
task.project = "test"
367402

368-
parent_job = SparkApplicationMaterializationJob("job1", "default", MagicMock())
369-
jobs = engine._build_per_fv_jobs(mock_registry, [task], "job1", parent_job)
403+
jobs = engine._build_per_fv_jobs(mock_registry, [task], "job1")
370404

371405
assert len(jobs) == 1
372-
assert jobs[0] is parent_job
373-
mock_registry.get_feature_view.assert_not_called()
406+
assert jobs[0].status() == MaterializationJobStatus.ERROR
407+
assert "fv_1" in str(jobs[0].error())
408+
engine.custom_api.get_namespaced_custom_object.assert_not_called()
374409

375410

376411
# ── Test 16: CompletedMaterializationJob is always SUCCEEDED ──

0 commit comments

Comments
 (0)