GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — feat: Apply cache to load proto registry for performance by j1wonpark · Pull Request #3702 · feast-dev/feast

Conversation

@j1wonpark

@j1wonpark j1wonpark commented Jul 28, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@feast-ci-bot

feast-ci-bot commented Jul 28, 2023

Copy link
Copy Markdown
Collaborator

[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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@feast-ci-bot feast-ci-bot added the size/S label Jul 28, 2023
@j1wonpark 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 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 feast-ci-bot added size/M and removed size/S labels Jul 28, 2023
@j1wonpark 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
@j1wonpark

j1wonpark commented Jul 31, 2023

Copy link
Copy Markdown
Contributor Author

/assign @zhilingc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
achals
achals reviewed Aug 28, 2023
View reviewed changes
Comment thread sdk/python/feast/infra/registry/proto_registry_utils.py
Comment on lines +39 to +40
else:
cache_value = func(registry_proto, project)

@achals achals Aug 28, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@j1wonpark j1wonpark Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread sdk/python/feast/infra/registry/proto_registry_utils.py
Comment on lines +27 to +29
def registry_proto_cache(func):
cache_key = None
cache_value = None

@achals achals Aug 28, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@j1wonpark j1wonpark Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
j1wonpark added 2 commits August 29, 2023 16:32
@j1wonpark
fix: Remove unused parameter
65659fe 
Signed-off-by: Jiwon Park <[email protected]>
@j1wonpark
feat: Apply cache to load proto registry for performance
f762ea2 
Signed-off-by: Jiwon Park <[email protected]>
@achals

achals commented Aug 29, 2023

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@j1wonpark
fix: Fix update key when cache missed
3ae34b7 
Signed-off-by: phil-park <[email protected]>
@j1wonpark

j1wonpark commented Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@j1wonpark

j1wonpark commented Sep 3, 2023

Copy link
Copy Markdown
Contributor Author

@achals Does this PR need further review?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
felixwang9817
felixwang9817 approved these changes Sep 12, 2023
View reviewed changes

@felixwang9817 felixwang9817 left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@felixwang9817 felixwang9817 added the lgtm label Sep 12, 2023
@felixwang9817
felixwang9817 merged commit 709c709 into feast-dev:master Sep 12, 2023
JohnLemmonMedely pushed a commit to JohnLemmonMedely/feast that referenced this pull request Sep 15, 2023
@j1wonpark @JohnLemmonMedely
feat: Apply cache to load proto registry for performance (feast-dev#3702
793a30f 
)

* fix: Remove unused parameter

Signed-off-by: Jiwon Park <[email protected]>

* feat: Apply cache to load proto registry for performance

Signed-off-by: Jiwon Park <[email protected]>

* fix: Fix update key when cache missed

Signed-off-by: phil-park <[email protected]>

---------

Signed-off-by: Jiwon Park <[email protected]>
Signed-off-by: phil-park <[email protected]>
@j1wonpark j1wonpark mentioned this pull request Dec 11, 2023
Feature retrieval latency scales with number of feature views, doesn't appear to cache #3442
Closed
woop pushed a commit that referenced this pull request Jan 13, 2024
@feast-ci-bot
chore(release): release 0.35.0
9237361 
# [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))
tokoko pushed a commit to tokoko/feast that referenced this pull request Feb 6, 2024
@feast-ci-bot @tokoko
chore(release): release 0.35.0
85b5b55 
# [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 <[email protected]>
zseta pushed a commit to zseta/feast that referenced this pull request Feb 7, 2024
@j1wonpark @zseta
feat: Apply cache to load proto registry for performance (feast-dev#3702
2c1f4b3 
)

* fix: Remove unused parameter

Signed-off-by: Jiwon Park <[email protected]>

* feat: Apply cache to load proto registry for performance

Signed-off-by: Jiwon Park <[email protected]>

* fix: Fix update key when cache missed

Signed-off-by: phil-park <[email protected]>

---------

Signed-off-by: Jiwon Park <[email protected]>
Signed-off-by: phil-park <[email protected]>
Signed-off-by: Attila Toth <[email protected]>
zseta pushed a commit to zseta/feast that referenced this pull request Feb 7, 2024
@feast-ci-bot @zseta
chore(release): release 0.35.0
ef5ff9e 
# [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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@achals achals achals left review comments
@felixwang9817 felixwang9817 felixwang9817 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

lgtm size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

performance issues of getting online features related to parsing protobuf data

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@j1wonpark @feast-ci-bot @achals @felixwang9817
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.