feat: Apply cache to load proto registry for performance by j1wonpark · Pull Request #3702 · feast-dev/feast

j1wonpark · 2023-07-28T05:03:21Z

What this PR does / why we need it:
If you use File Registry, the registry file becomes heavy because materialize records are accumulated while a long period.
Even if the registry cache is enabled, performance degraded because each online feature lookup traverses the registry object and searches the meta.

As long as the registry_proto object does not change, you must avoid multiple lookups through the function-level cache.

Which issue(s) this PR fixes:

Fixes #3649 #3597

feast-ci-bot · 2023-07-28T05:03:24Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: phil-park
To complete the pull request process, please assign zhilingc after the PR has been reviewed.
You can assign the PR to them by writing /assign @zhilingc in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

j1wonpark · 2023-07-31T00:29:19Z

/assign @zhilingc

achals · 2023-08-28T18:04:39Z

sdk/python/feast/infra/registry/proto_registry_utils.py

+        else:
+            cache_value = func(registry_proto, project)


Don't you need to update cache_key here as well?

@achals I'm sorry, you're right. I added commit b8a373f

achals · 2023-08-28T18:14:01Z

sdk/python/feast/infra/registry/proto_registry_utils.py

+def registry_proto_cache(func):
+    cache_key = None
+    cache_value = None


I'm not sure I understand this.. If you use the same annotation for multiple methods then the cache value needs have different shapes, such as List[FeatureService] or List[FeatureView] right? Is that going to work as expected here? Can we add tests?

The context of the wrapper decorator is created whenever different argument functions (e.g list_feature_views, list_feature_services..) are called.
Therefore, when calling list_feature_views and when calling list_feature_services, different results are returned.
The operation of this cache is similar to Python's functools.lru_cache, but since the registryProto object is not hashable, it is implemented to hash using the object's id.

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>

achals · 2023-08-29T17:37:40Z

Okay, can you also sign off your commits? https://github.com/feast-dev/feast/pull/3702/checks?check_run_id=16298395815

Signed-off-by: phil-park <bakjeeone@hotmail.com>

j1wonpark · 2023-08-29T23:59:46Z

I signed off the commit. Thank you for your review.

j1wonpark · 2023-09-03T22:35:37Z

@achals Does this PR need further review?

felixwang9817

/lgtm

) * fix: Remove unused parameter Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> * feat: Apply cache to load proto registry for performance Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> * fix: Fix update key when cache missed Signed-off-by: phil-park <bakjeeone@hotmail.com> --------- Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> Signed-off-by: phil-park <bakjeeone@hotmail.com>

# [0.35.0](v0.34.0...v0.35.0) (2024-01-13) ### Bug Fixes * Add async refresh to prevent synchronous refresh in main thread ([#3812](#3812)) ([9583ed6](9583ed6)) * Adopt connection pooling for HBase ([#3793](#3793)) ([b3852bf](b3852bf)) * Bytewax engine create configmap from object ([#3821](#3821)) ([25e9775](25e9775)) * Fix warnings from deprecated paths and update default log level ([#3757](#3757)) ([68a8737](68a8737)) * improve parsing bytewax job status ([5983f40](5983f40)) * make bytewax settings unexposed ([ae1bb8b](ae1bb8b)) * Make generated temp table name escaped ([#3797](#3797)) ([175d796](175d796)) * Pin numpy version to avoid spammy deprecation messages ([774ed33](774ed33)) * Redundant feature materialization and premature incremental materialization timestamp updates ([#3789](#3789)) ([417b16b](417b16b)), closes [#6](#6) [#7](#7) * Resolve hbase hotspot issue when materializing ([#3790](#3790)) ([7376db8](7376db8)) * Set keepalives_idle None by default ([#3756](#3756)) ([8717e9b](8717e9b)) * Set upper bound for bigquery client due to its breaking changes ([2151c39](2151c39)) * UI project cannot handle fallback routes ([#3766](#3766)) ([96ece0f](96ece0f)) * update dependencies versions due to conflicts ([5dc0b24](5dc0b24)) * Update jackson and remove unnecessary logging ([#3809](#3809)) ([018d0ea](018d0ea)) * upgrade the pyarrow to latest v14.0.1 for CVE-2023-47248. ([052182b](052182b)) ### Features * Add get online feature rpc to gprc server ([#3815](#3815)) ([01db8cc](01db8cc)) * Add materialize and materialize-incremental rest endpoints ([#3761](#3761)) ([fa600fe](fa600fe)), closes [#3760](#3760) * add redis sentinel support ([3387a15](3387a15)) * add redis sentinel support ([4337c89](4337c89)) * add redis sentinel support format lint ([aad8718](aad8718)) * Add support for `table_create_disposition` in bigquery job for offline store ([#3762](#3762)) ([6a728fe](6a728fe)) * Add support for in_cluster config and additional labels for bytewax materialization ([#3754](#3754)) ([2192e65](2192e65)) * Apply cache to load proto registry for performance ([#3702](#3702)) ([709c709](709c709)) * Make bytewax job write as mini-batches ([#3777](#3777)) ([9b0e5ce](9b0e5ce)) * Optimize bytewax pod resource with zero-copy ([9cf9d96](9cf9d96)) * Support GCS filesystem for bytewax engine ([#3774](#3774)) ([fb6b807](fb6b807))

# [0.35.0](feast-dev/feast@v0.34.0...v0.35.0) (2024-01-13) ### Bug Fixes * Add async refresh to prevent synchronous refresh in main thread ([feast-dev#3812](feast-dev#3812)) ([9583ed6](feast-dev@9583ed6)) * Adopt connection pooling for HBase ([feast-dev#3793](feast-dev#3793)) ([b3852bf](feast-dev@b3852bf)) * Bytewax engine create configmap from object ([feast-dev#3821](feast-dev#3821)) ([25e9775](feast-dev@25e9775)) * Fix warnings from deprecated paths and update default log level ([feast-dev#3757](feast-dev#3757)) ([68a8737](feast-dev@68a8737)) * improve parsing bytewax job status ([5983f40](feast-dev@5983f40)) * make bytewax settings unexposed ([ae1bb8b](feast-dev@ae1bb8b)) * Make generated temp table name escaped ([feast-dev#3797](feast-dev#3797)) ([175d796](feast-dev@175d796)) * Pin numpy version to avoid spammy deprecation messages ([774ed33](feast-dev@774ed33)) * Redundant feature materialization and premature incremental materialization timestamp updates ([feast-dev#3789](feast-dev#3789)) ([417b16b](feast-dev@417b16b)), closes [feast-dev#6](feast-dev#6) [feast-dev#7](feast-dev#7) * Resolve hbase hotspot issue when materializing ([feast-dev#3790](feast-dev#3790)) ([7376db8](feast-dev@7376db8)) * Set keepalives_idle None by default ([feast-dev#3756](feast-dev#3756)) ([8717e9b](feast-dev@8717e9b)) * Set upper bound for bigquery client due to its breaking changes ([2151c39](feast-dev@2151c39)) * UI project cannot handle fallback routes ([feast-dev#3766](feast-dev#3766)) ([96ece0f](feast-dev@96ece0f)) * update dependencies versions due to conflicts ([5dc0b24](feast-dev@5dc0b24)) * Update jackson and remove unnecessary logging ([feast-dev#3809](feast-dev#3809)) ([018d0ea](feast-dev@018d0ea)) * upgrade the pyarrow to latest v14.0.1 for CVE-2023-47248. ([052182b](feast-dev@052182b)) ### Features * Add get online feature rpc to gprc server ([feast-dev#3815](feast-dev#3815)) ([01db8cc](feast-dev@01db8cc)) * Add materialize and materialize-incremental rest endpoints ([feast-dev#3761](feast-dev#3761)) ([fa600fe](feast-dev@fa600fe)), closes [feast-dev#3760](feast-dev#3760) * add redis sentinel support ([3387a15](feast-dev@3387a15)) * add redis sentinel support ([4337c89](feast-dev@4337c89)) * add redis sentinel support format lint ([aad8718](feast-dev@aad8718)) * Add support for `table_create_disposition` in bigquery job for offline store ([feast-dev#3762](feast-dev#3762)) ([6a728fe](feast-dev@6a728fe)) * Add support for in_cluster config and additional labels for bytewax materialization ([feast-dev#3754](feast-dev#3754)) ([2192e65](feast-dev@2192e65)) * Apply cache to load proto registry for performance ([feast-dev#3702](feast-dev#3702)) ([709c709](feast-dev@709c709)) * Make bytewax job write as mini-batches ([feast-dev#3777](feast-dev#3777)) ([9b0e5ce](feast-dev@9b0e5ce)) * Optimize bytewax pod resource with zero-copy ([9cf9d96](feast-dev@9cf9d96)) * Support GCS filesystem for bytewax engine ([feast-dev#3774](feast-dev#3774)) ([fb6b807](feast-dev@fb6b807)) Signed-off-by: tokoko <togurg14@freeuni.edu.ge>

) * fix: Remove unused parameter Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> * feat: Apply cache to load proto registry for performance Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> * fix: Fix update key when cache missed Signed-off-by: phil-park <bakjeeone@hotmail.com> --------- Signed-off-by: Jiwon Park <bakjeeone@hotmail.com> Signed-off-by: phil-park <bakjeeone@hotmail.com> Signed-off-by: Attila Toth <hello@attilatoth.dev>

# [0.35.0](feast-dev/feast@v0.34.0...v0.35.0) (2024-01-13) ### Bug Fixes * Add async refresh to prevent synchronous refresh in main thread ([feast-dev#3812](feast-dev#3812)) ([9583ed6](feast-dev@9583ed6)) * Adopt connection pooling for HBase ([feast-dev#3793](feast-dev#3793)) ([b3852bf](feast-dev@b3852bf)) * Bytewax engine create configmap from object ([feast-dev#3821](feast-dev#3821)) ([25e9775](feast-dev@25e9775)) * Fix warnings from deprecated paths and update default log level ([feast-dev#3757](feast-dev#3757)) ([68a8737](feast-dev@68a8737)) * improve parsing bytewax job status ([5983f40](feast-dev@5983f40)) * make bytewax settings unexposed ([ae1bb8b](feast-dev@ae1bb8b)) * Make generated temp table name escaped ([feast-dev#3797](feast-dev#3797)) ([175d796](feast-dev@175d796)) * Pin numpy version to avoid spammy deprecation messages ([774ed33](feast-dev@774ed33)) * Redundant feature materialization and premature incremental materialization timestamp updates ([feast-dev#3789](feast-dev#3789)) ([417b16b](feast-dev@417b16b)), closes [feast-dev#6](feast-dev#6) [feast-dev#7](feast-dev#7) * Resolve hbase hotspot issue when materializing ([feast-dev#3790](feast-dev#3790)) ([7376db8](feast-dev@7376db8)) * Set keepalives_idle None by default ([feast-dev#3756](feast-dev#3756)) ([8717e9b](feast-dev@8717e9b)) * Set upper bound for bigquery client due to its breaking changes ([2151c39](feast-dev@2151c39)) * UI project cannot handle fallback routes ([feast-dev#3766](feast-dev#3766)) ([96ece0f](feast-dev@96ece0f)) * update dependencies versions due to conflicts ([5dc0b24](feast-dev@5dc0b24)) * Update jackson and remove unnecessary logging ([feast-dev#3809](feast-dev#3809)) ([018d0ea](feast-dev@018d0ea)) * upgrade the pyarrow to latest v14.0.1 for CVE-2023-47248. ([052182b](feast-dev@052182b)) ### Features * Add get online feature rpc to gprc server ([feast-dev#3815](feast-dev#3815)) ([01db8cc](feast-dev@01db8cc)) * Add materialize and materialize-incremental rest endpoints ([feast-dev#3761](feast-dev#3761)) ([fa600fe](feast-dev@fa600fe)), closes [feast-dev#3760](feast-dev#3760) * add redis sentinel support ([3387a15](feast-dev@3387a15)) * add redis sentinel support ([4337c89](feast-dev@4337c89)) * add redis sentinel support format lint ([aad8718](feast-dev@aad8718)) * Add support for `table_create_disposition` in bigquery job for offline store ([feast-dev#3762](feast-dev#3762)) ([6a728fe](feast-dev@6a728fe)) * Add support for in_cluster config and additional labels for bytewax materialization ([feast-dev#3754](feast-dev#3754)) ([2192e65](feast-dev@2192e65)) * Apply cache to load proto registry for performance ([feast-dev#3702](feast-dev#3702)) ([709c709](feast-dev@709c709)) * Make bytewax job write as mini-batches ([feast-dev#3777](feast-dev#3777)) ([9b0e5ce](feast-dev@9b0e5ce)) * Optimize bytewax pod resource with zero-copy ([9cf9d96](feast-dev@9cf9d96)) * Support GCS filesystem for bytewax engine ([feast-dev#3774](feast-dev#3774)) ([fb6b807](feast-dev@fb6b807)) Signed-off-by: Attila Toth <hello@attilatoth.dev>

feast-ci-bot added the size/S label Jul 28, 2023

j1wonpark changed the title ~~Apply lru_cache to retrieve registry for performance~~ feat: Apply lru_cache to retrieve regfeistry for performance Jul 28, 2023

j1wonpark changed the title ~~feat: Apply lru_cache to retrieve regfeistry for performance~~ feat: Apply lru_cache to retrieve registry for performance Jul 28, 2023

feast-ci-bot added size/M and removed size/S labels Jul 28, 2023

j1wonpark changed the title ~~feat: Apply lru_cache to retrieve registry for performance~~ feat: Apply cache to load proto registry for performance Jul 28, 2023

achals reviewed Aug 28, 2023

View reviewed changes

j1wonpark added 2 commits August 29, 2023 16:32

fix: Remove unused parameter

65659fe

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>

feat: Apply cache to load proto registry for performance

f762ea2

Signed-off-by: Jiwon Park <bakjeeone@hotmail.com>

fix: Fix update key when cache missed

3ae34b7

Signed-off-by: phil-park <bakjeeone@hotmail.com>

felixwang9817 approved these changes Sep 12, 2023

View reviewed changes

felixwang9817 added the lgtm label Sep 12, 2023

felixwang9817 merged commit 709c709 into feast-dev:master Sep 12, 2023

j1wonpark mentioned this pull request Dec 11, 2023

Feature retrieval latency scales with number of feature views, doesn't appear to cache #3442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Apply cache to load proto registry for performance#3702

feat: Apply cache to load proto registry for performance#3702
felixwang9817 merged 3 commits intofeast-dev:masterfrom
j1wonpark:master

j1wonpark commented Jul 28, 2023 •

edited

Loading

Uh oh!

feast-ci-bot commented Jul 28, 2023

Uh oh!

j1wonpark commented Jul 31, 2023

Uh oh!

achals Aug 28, 2023

Uh oh!

j1wonpark Aug 29, 2023

Uh oh!

achals Aug 28, 2023

Uh oh!

j1wonpark Aug 29, 2023

Uh oh!

achals commented Aug 29, 2023

Uh oh!

j1wonpark commented Aug 29, 2023

Uh oh!

j1wonpark commented Sep 3, 2023

Uh oh!

felixwang9817 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

j1wonpark commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feast-ci-bot commented Jul 28, 2023

Uh oh!

j1wonpark commented Jul 31, 2023

Uh oh!

achals Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

j1wonpark Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

achals Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

j1wonpark Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

achals commented Aug 29, 2023

Uh oh!

j1wonpark commented Aug 29, 2023

Uh oh!

j1wonpark commented Sep 3, 2023

Uh oh!

felixwang9817 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

j1wonpark commented Jul 28, 2023 •

edited

Loading