fix: Resolve write_to_offline_store feature view with a single registry lookup - #6696
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6696 +/- ##
==========================================
+ Coverage 46.91% 47.00% +0.08%
==========================================
Files 416 416
Lines 50545 50600 +55
Branches 7252 7262 +10
==========================================
+ Hits 23714 23783 +69
+ Misses 25157 25129 -28
- Partials 1674 1688 +14
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
franciscojavierarceo
left a comment
There was a problem hiding this comment.
The unified get_any_feature_view lookup preserves the supported feature-view types while removing guaranteed-miss RPCs on the remote-registry write path, and the regression asserts the single lookup. I found no blocking issue in the remote diff.
0e63266 to
f148010
Compare
…ry lookup write_to_offline_store resolved the feature view with a try/except chain that called get_stream_feature_view, then get_feature_view, then get_label_view in turn. A plain FeatureView -- the common case -- never matches the first lookup, so it was always issued, failed with FeatureViewNotFoundException, and discarded before the lookup that could succeed ran. On a RemoteRegistry each attempt is a gRPC round-trip to the registry server (no client-side cache), so every batch on this per-batch write path paid one or more guaranteed-miss RPCs before the real one. allow_registry_cache=True, the default, does not avoid it on a remote registry. Resolve the feature view with a single registry.get_any_feature_view lookup instead -- the unified accessor added in feast-dev#4235 for exactly the case where a caller holds only a name. It covers FeatureView, StreamFeatureView, OnDemandFeatureView, and LabelView, so it is a behaviour-preserving replacement for the chain while collapsing up to three registry lookups into one. Fixes feast-dev#6671. Signed-off-by: adarshsm <24850536+adarshsm@users.noreply.github.com>
f148010 to
15888cd
Compare
What this does
FeatureStore.write_to_offline_storeresolved the feature view with a try/except chain (sdk/python/feast/feature_store.py):For a plain
FeatureView— the common case — the first lookup can never succeed. It is issued, raisesFeatureViewNotFoundException, and is discarded, and only then does the lookup that can succeed run. ALabelViewpays two failed lookups before the third.Every getter forwards
allow_registry_cacheas the registry'sallow_cache. On aCachingRegistrythe failed attempts come from the cached proto and cost little. On aRemoteRegistryeach is a gRPC round-trip to the registry server (it keeps no client-side cache and forwardsallow_cacheto the server as a request field). This sits on a per-batch write path, so a backfill pays a guaranteed-miss registry RPC before the real one on every batch, andallow_registry_cache=True(the default) does not avoid it.The fix
Resolve the feature view with a single
registry.get_any_feature_view(name, project, allow_cache=...)lookup — the unified accessor added in #4235 for exactly the case where a caller holds only a name. It coversFeatureView,StreamFeatureView,OnDemandFeatureView, andLabelView, so it is a behaviour-preserving replacement for the chain while collapsing up to three registry lookups into one.#4235introducedget_any_feature_viewto address this same three-getters asymmetry inBaseRegistryand was closed as completed, but this call site was never migrated — it still performed the try/except chain, and a third branch (get_label_view) has been added since.Testing
Added
test_write_to_offline_store_resolves_feature_view_with_single_lookup(sdk/python/tests/unit/test_unit_feature_store.py): it asserts exactly oneget_any_feature_viewcall and that none of the legacy per-type getters fire. The test fails againstmaster(the chain callsget_stream_feature_view, notget_any_feature_view) and passes with this change.ruff check/ruff formatclean on both files;mypyclean onfeature_store.py.Fixes #6671.