feat: Add Async refresh to Sql Registry#4251

stanconia

What this PR does / why we need it:

Support Async refresh in Sql Registry to facilitate Registry functions at an Enterprise level

Which issue(s) this PR fixes:

Registry Refresh at an enterprise scale

Fixes

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia

ok-to-test

tokoko

I'm still not sure if I'm opposed to the idea, but just to note: don't we already do something like this "externally", for example a python feature server has an async methods that refresh registry in the background?

stanconia

I'm still not sure if I'm opposed to the idea, but just to note: don't we already do something like this "externally", for example a python feature server has an async methods that refresh registry in the background?

@tokoko the problem with async refresh changes in Feature Server is, when multiple feature servers perform async refreshes Registry will be overloaded which does not scale at an enterprise leve. We believe aync refresh should be done at the cacheRegistry level.

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

tokoko

@stanconia I don't particularly buy that argument tbh. If you're running multiple feature servers, each will almost certainly be running in it's own process/container, so each will come with it's own CachingRegistry. The behavior will be exactly the same from a load perspective. Having said that, decoupling all that from feature server and making it more reusable might be a good idea. I'll have to take a closer look at the components involved first and get back to you (feature server, ui server, caching registry, ...)

stanconia

@stanconia I don't particularly buy that argument tbh. If you're running multiple feature servers, each will almost certainly be running in it's own process/container, so each will come with it's own CachingRegistry. The behavior will be exactly the same from a load perspective. Having said that, decoupling all that from feature server and making it more reusable might be a good idea. I'll have to take a closer look at the components involved first and get back to you (feature server, ui server, caching registry, ...)

You are right for some usecases but in our usecase we are using a Remote Registry proxied to a Sql Registry.

tokoko

@stanconia oh, great. makes sense. didn't know anyone was already using it, I still have to add remote registry to the docs :)

stanconia

@stanconia oh, great. makes sense. didn't know anyone was already using it, I still have to add remote registry to the docs :)

@tokoko gentle reminder on reviewing/considering this pr.

tokoko

How about instead of allow_async_cache, we introduce cache_mode config that can be sync (default), thread (implementation in this PR) and in the future possibly async (similar to this, but with asyncio). async referring to a separate thread in python is a little confusing at first.

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia

How about instead of allow_async_cache, we introduce cache_mode config that can be sync (default), thread (implementation in this PR) and in the future possibly async (similar to this, but with asyncio). async referring to a separate thread in python is a little confusing at first.

@tokoko requested change has been implemented.

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia

@tokoko please can you review/approve pr if there are no other concerns. Thanks

tokoko

@jeremyary the tests need approval here

stanconia

@jeremyary the tests need approval here

@jeremyary any updates? cc @tokoko

stanconia

@jeremyary the tests need approval here

@jeremyary any updates? cc @tokoko

@tokoko can you assist here?

tokoko

@franciscojavierarceo @HaoXuAI can you help here? Tests need a first-time contributor approval.

HaoXuAI

hey, sorry merged amother PRs and caused conflicts. mind resolve it? I'll follow up from there.

stanconia

hey, sorry merged amother PRs and caused conflicts. mind resolve it? I'll follow up from there.

@HaoXuAI resolved conflicts please approve

HaoXuAI

lgtm

stanconia

lgtm

@tokoko please review.

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

tokoko

lgtm

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia

2 workflows awaiting approval

@tokoko how do i get approvals for pending workflows?

franciscojavierarceo

@tokoko can you try /lgtm

tokoko

/lgtm

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia

/lgtm

@tokoko please approve lint workflows

stanconia

/lgtm

@tokoko please approve lint workflows

@tokoko please can you assist with a merge

tokoko

@stanconia I don't have merge rights, Francisco can merge as well

* Add sql registry async refresh Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * make refresh code a daemon thread Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Change RegistryConfig to cacheMode Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Only run async when ttl > 0 Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * make refresh async run in a loop Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * make refresh async run in a loop Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Reorder async refresh call Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Add documentation Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Update test_universal_registry.py Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Force rerun of tests Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Force rerun of tests Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> * Format repo config file Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> --------- Signed-off-by: Stanley Opara <a-sopara@expediagroup.com> Co-authored-by: Stanley Opara <a-sopara@expediagroup.com>

# [0.40.0](v0.39.0...v0.40.0) (2024-07-31) ### Bug Fixes * Added missing type ([#4315](#4315)) ([86af60a](86af60a)) * Avoid XSS attack from Jinjin2's Environment(). ([#4355](#4355)) ([40270e7](40270e7)) * CGO Memory leak issue in GO Feature server ([#4291](#4291)) ([43e198f](43e198f)) * Deprecated the datetime.utcfromtimestamp(). ([#4306](#4306)) ([21deec8](21deec8)) * Fix SQLite import issue ([#4294](#4294)) ([398ea3b](398ea3b)) * Increment operator to v0.39.0 ([#4368](#4368)) ([3ddb4fb](3ddb4fb)) * Minor typo in the unit test. ([#4296](#4296)) ([6c75e84](6c75e84)) * OnDemandFeatureView type inference for array types ([#4310](#4310)) ([c45ff72](c45ff72)) * Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([#4331](#4331)) ([0d89d15](0d89d15)) * Remove typo. ([#4351](#4351)) ([92d17de](92d17de)) * Retire the datetime.utcnow(). ([#4352](#4352)) ([a8bc696](a8bc696)) * Update dask version to support pandas 1.x ([#4326](#4326)) ([a639d61](a639d61)) * Update Feast object metadata in the registry ([#4257](#4257)) ([8028ae0](8028ae0)) * Using one single function call for utcnow(). ([#4307](#4307)) ([98ff63c](98ff63c)) ### Features * Add async feature retrieval for Postgres Online Store ([#4327](#4327)) ([cea52e9](cea52e9)) * Add Async refresh to Sql Registry ([#4251](#4251)) ([f569786](f569786)) * Add SingleStore as an OnlineStore ([#4285](#4285)) ([2c38946](2c38946)) * Add Tornike to maintainers.md ([#4339](#4339)) ([8e8c1f2](8e8c1f2)) * Bump psycopg2 to psycopg3 for all Postgres components ([#4303](#4303)) ([9451d9c](9451d9c)) * Entity key deserialization ([#4284](#4284)) ([83fad15](83fad15)) * Ignore paths feast apply ([#4276](#4276)) ([b4d54af](b4d54af)) * Move get_online_features to OnlineStore interface ([#4319](#4319)) ([7072fd0](7072fd0)) * Port mssql contrib offline store to ibis ([#4360](#4360)) ([7914cbd](7914cbd)) ### Reverts * Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([#4357](#4357)) ([cdeab48](cdeab48)), closes [#4355](#4355)

# [0.40.0](feast-dev/feast@v0.39.0...v0.40.0) (2024-07-31) ### Bug Fixes * Added missing type ([feast-dev#4315](feast-dev#4315)) ([86af60a](feast-dev@86af60a)) * Avoid XSS attack from Jinjin2's Environment(). ([feast-dev#4355](feast-dev#4355)) ([40270e7](feast-dev@40270e7)) * CGO Memory leak issue in GO Feature server ([feast-dev#4291](feast-dev#4291)) ([43e198f](feast-dev@43e198f)) * Deprecated the datetime.utcfromtimestamp(). ([feast-dev#4306](feast-dev#4306)) ([21deec8](feast-dev@21deec8)) * Fix SQLite import issue ([feast-dev#4294](feast-dev#4294)) ([398ea3b](feast-dev@398ea3b)) * Increment operator to v0.39.0 ([feast-dev#4368](feast-dev#4368)) ([3ddb4fb](feast-dev@3ddb4fb)) * Minor typo in the unit test. ([feast-dev#4296](feast-dev#4296)) ([6c75e84](feast-dev@6c75e84)) * OnDemandFeatureView type inference for array types ([feast-dev#4310](feast-dev#4310)) ([c45ff72](feast-dev@c45ff72)) * Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([feast-dev#4331](feast-dev#4331)) ([0d89d15](feast-dev@0d89d15)) * Remove typo. ([feast-dev#4351](feast-dev#4351)) ([92d17de](feast-dev@92d17de)) * Retire the datetime.utcnow(). ([feast-dev#4352](feast-dev#4352)) ([a8bc696](feast-dev@a8bc696)) * Update dask version to support pandas 1.x ([feast-dev#4326](feast-dev#4326)) ([a639d61](feast-dev@a639d61)) * Update Feast object metadata in the registry ([feast-dev#4257](feast-dev#4257)) ([8028ae0](feast-dev@8028ae0)) * Using one single function call for utcnow(). ([feast-dev#4307](feast-dev#4307)) ([98ff63c](feast-dev@98ff63c)) ### Features * Add async feature retrieval for Postgres Online Store ([feast-dev#4327](feast-dev#4327)) ([cea52e9](feast-dev@cea52e9)) * Add Async refresh to Sql Registry ([feast-dev#4251](feast-dev#4251)) ([f569786](feast-dev@f569786)) * Add SingleStore as an OnlineStore ([feast-dev#4285](feast-dev#4285)) ([2c38946](feast-dev@2c38946)) * Add Tornike to maintainers.md ([feast-dev#4339](feast-dev#4339)) ([8e8c1f2](feast-dev@8e8c1f2)) * Bump psycopg2 to psycopg3 for all Postgres components ([feast-dev#4303](feast-dev#4303)) ([9451d9c](feast-dev@9451d9c)) * Entity key deserialization ([feast-dev#4284](feast-dev#4284)) ([83fad15](feast-dev@83fad15)) * Ignore paths feast apply ([feast-dev#4276](feast-dev#4276)) ([b4d54af](feast-dev@b4d54af)) * Move get_online_features to OnlineStore interface ([feast-dev#4319](feast-dev#4319)) ([7072fd0](feast-dev@7072fd0)) * Port mssql contrib offline store to ibis ([feast-dev#4360](feast-dev#4360)) ([7914cbd](feast-dev@7914cbd)) ### Reverts * Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([feast-dev#4357](feast-dev#4357)) ([cdeab48](feast-dev@cdeab48)), closes [feast-dev#4355](feast-dev#4355)

franciscojavierarceo

Hi @stanconia I was going to give you a shoutout in the latest food newsletter if you wanted on. Feel free to add your name here: https://docs.google.com/document/d/1sKD1Lt3h4G9vzitbgoOwNX9uOrLCjdzw4bJnUIO4ixU/edit#bookmark=id.909bt66x3owh

stanconia

Thanks. I don't have access to write my name but please add, Stanley Opara

…

On Mon, Aug 26, 2024, 11:39 PM Francisco Arceo ***@***.***> wrote: Hi @stanconia <https://github.com/stanconia> I was going to give you a shoutout in the latest food newsletter if you wanted on. Feel free to add your name here: https://docs.google.com/document/d/1sKD1Lt3h4G9vzitbgoOwNX9uOrLCjdzw4bJnUIO4ixU/edit#bookmark=id.909bt66x3owh — Reply to this email directly, view it on GitHub <#4251 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIXBQJO2VZH5MGM5BFCQOGLZTPYF5AVCNFSM6AAAAABIXIU5E6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJRGUYDKOBWHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Add sql registry async refresh …

c05b0db

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

make refresh code a daemon thread …

2074959

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia force-pushed the support_async_refresh branch from 8ca8939 to 2074959 Compare June 5, 2024 21:06

Change RegistryConfig to cacheMode …

956b759

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

tokoko reviewed Jun 11, 2024

View reviewed changes

Only run async when ttl > 0 …

da82ba0

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

tokoko reviewed Jun 12, 2024

View reviewed changes

Stanley Opara added 3 commits June 12, 2024 15:55

make refresh async run in a loop …

d4f58b6

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

make refresh async run in a loop …

3092823

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

Reorder async refresh call …

9a5cc16

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia force-pushed the support_async_refresh branch from a264d64 to 9a5cc16 Compare June 12, 2024 19:56

jeremyary added the ok-to-test label Jun 13, 2024

EXPEbdodla reviewed Jun 14, 2024

View reviewed changes

Add documentation …

4593127

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

Merge branch 'master' into support_async_refresh

16c3646

HaoXuAI approved these changes Jul 2, 2024

View reviewed changes

Merge branch 'master' into support_async_refresh

a8513f8

Update test_universal_registry.py …

8798696

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

stanconia force-pushed the support_async_refresh branch from e3b9483 to 8798696 Compare July 2, 2024 17:47

Force rerun of tests …

00a1501

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

tokoko approved these changes Jul 3, 2024

View reviewed changes

Force rerun of tests …

3195382

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

Format repo config file …

b7df692

Signed-off-by: Stanley Opara <a-sopara@expediagroup.com>

franciscojavierarceo approved these changes Jul 8, 2024

View reviewed changes

franciscojavierarceo merged commit f569786 into feast-dev:master Jul 9, 2024

Conversation

stanconia commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Uh oh!

stanconia commented Jun 4, 2024

Uh oh!

tokoko commented Jun 4, 2024

Uh oh!

stanconia commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tokoko commented Jun 6, 2024

Uh oh!

stanconia commented Jun 6, 2024

Uh oh!

tokoko commented Jun 8, 2024

Uh oh!

stanconia commented Jun 10, 2024

Uh oh!

tokoko commented Jun 10, 2024

Uh oh!

stanconia commented Jun 11, 2024

Uh oh!

stanconia commented Jun 17, 2024

Uh oh!

tokoko commented Jun 20, 2024

Uh oh!

stanconia commented Jun 21, 2024

Uh oh!

stanconia commented Jun 25, 2024

Uh oh!

tokoko commented Jun 25, 2024

Uh oh!

HaoXuAI commented Jul 1, 2024

Uh oh!

stanconia commented Jul 2, 2024

Uh oh!

HaoXuAI left a comment

Choose a reason for hiding this comment

Uh oh!

stanconia commented Jul 2, 2024

Uh oh!

tokoko left a comment

Choose a reason for hiding this comment

Uh oh!

stanconia commented Jul 3, 2024

Uh oh!

franciscojavierarceo commented Jul 4, 2024

Uh oh!

tokoko commented Jul 4, 2024

Uh oh!

stanconia commented Jul 8, 2024

Uh oh!

stanconia commented Jul 9, 2024

Uh oh!

tokoko commented Jul 9, 2024

Uh oh!

franciscojavierarceo commented Aug 27, 2024

Uh oh!

stanconia commented Aug 27, 2024 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

stanconia commented Jun 3, 2024 •

edited

Loading

stanconia commented Jun 5, 2024 •

edited

Loading