Repository navigation
perf: Key registry proto caches on version_id so a refresh does not rebuild objects - #7073
chandlerok wants to merge 2 commits into
Conversation
…ebuild objects The registry_proto_cache decorators keyed on id(registry_proto) as well as version_id. Every registry refresh parses a new proto object, so each refresh missed the cache and rebuilt every object from proto even when the registry had not changed. For a registry with 14 feature services that rebuild costs about 35 ms, most of it typeguard checks in Field.__init__. Key on version_id alone, falling back to the object identity only when a registry does not set a version_id. Also cache get_feature_service per version, which get_online_features calls on every request. Refs feast-dev#7072 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chandler King <chandler.king@findheadway.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7073 +/- ##
==========================================
+ Coverage 50.97% 51.00% +0.03%
==========================================
Files 455 455
Lines 56994 57001 +7
Branches 8321 8321
==========================================
+ Hits 29055 29076 +21
+ Misses 25954 25940 -14
Partials 1985 1985
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
…sion_id SqlRegistry.proto() and SnowflakeRegistry.proto() never set version_id, so the version-keyed caches fell back to object identity and every cache refresh still rebuilt every object from proto. Set version_id to the sha1 of the deterministic serialization, which is stable across refreshes and changes on any apply or delete. On a 421 KB registry proto the hash costs 0.2 ms. Refs feast-dev#7072 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chandler King <chandler.king@findheadway.com>
|
@ntkathole this follows on from #6870 and #7072. Two small commits: version-keyed registry caches, and a version_id on the SQL and Snowflake registry protos so those registries get the same cache hits. Numbers and profile are in the description. |
1 similar comment
|
@ntkathole this follows on from #6870 and #7072. Two small commits: version-keyed registry caches, and a version_id on the SQL and Snowflake registry protos so those registries get the same cache hits. Numbers and profile are in the description. |
What this PR does / why we need it:
The
registry_proto_cachedecorators inproto_registry_utilskeyed onid(registry_proto)as well asversion_id. A registry refresh parses a new proto object, so every refresh missed the cache and rebuilt every object from proto even when the registry had not changed. With 14 feature services that rebuild is about 35 ms per refresh, most of it typeguard checks inField.__init__. Measured locally with a file registry, refresh pluslist_feature_services(allow_cache=True):Changes:
version_idalone. A registry without aversion_idfalls back to object identity, which is today's behavior.get_feature_serviceper version with the existingregistry_proto_cache_namedecorator.get_online_featurescalls it on every request and it rebuilt theFeatureServicefrom proto each time, about 2 ms per call.SqlRegistry.proto()andSnowflakeRegistry.proto()with aversion_id. Neither set one, so for those registries the caches would have kept falling back to object identity and missing on every refresh. The value is the sha1 of the deterministic serialization, stable across refreshes and different after any apply or delete. On a 421 KB proto the hash costs 0.2 ms. The file, S3, GCS, Azure, and HDFS stores already stamp a uuid on each write.Profiled effect on
get_online_featureswith a file registry, 100 entities, 4 feature views: a warm read has nofrom_protoor typeguard frames left on the registry side. The first read after a refresh costs 14.2 ms against 19.6 ms with the old object-identity key, and the same 5 ms saving now applies to SQL-registry deployments once per cache refresh.No behavior change for callers. Cached objects were already shared between calls on the same proto object.
Which issue(s) this PR fixes:
Refs #7072 (proposal 3)
Checks
git commit -s)Testing Strategy
Unit tests cover a re-parsed proto with the same
version_idhitting the cache, a newversion_idmissing it, registries without aversion_idstaying per-object,get_feature_servicereturning the cached object, and a SQL registry whoseversion_idis stable acrossproto()calls and refreshes and changes after an apply.sdk/python/tests/unit/infra/registrypasses locally (164 tests; three modules skip collection for missing optional dependencies).