feat: Add Async refresh to Sql Registry #4251

stanconia · 2024-06-03T21:22:26Z

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 · 2024-06-04T03:22:44Z

ok-to-test

tokoko · 2024-06-04T13:33:26Z

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 · 2024-06-05T19:33:57Z

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 · 2024-06-06T08:42:05Z

@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 · 2024-06-06T15:40:13Z

@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 · 2024-06-08T07:31:50Z

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

stanconia · 2024-06-10T13:52:25Z

@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 · 2024-06-10T20:52:12Z

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 · 2024-06-11T20:08:56Z

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.

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

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

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

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

sdk/python/feast/repo_config.py

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

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

stanconia · 2024-06-17T19:41:49Z

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

tokoko · 2024-06-20T05:17:33Z

@jeremyary the tests need approval here

stanconia · 2024-06-21T15:34:40Z

@jeremyary the tests need approval here

@jeremyary any updates? cc @tokoko

stanconia · 2024-06-25T14:23:09Z

@jeremyary the tests need approval here

@jeremyary any updates? cc @tokoko

@tokoko can you assist here?

tokoko · 2024-06-25T18:56:28Z

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

HaoXuAI · 2024-07-01T21:08:45Z

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

stanconia · 2024-07-02T02:35:45Z

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 · 2024-07-02T14:48:42Z

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 · 2024-07-03T18:03:21Z

2 workflows awaiting approval

@tokoko how do i get approvals for pending workflows?

franciscojavierarceo · 2024-07-04T14:50:18Z

@tokoko can you try /lgtm

tokoko · 2024-07-04T19:50:46Z

/lgtm

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

stanconia · 2024-07-08T15:27:47Z

/lgtm

@tokoko please approve lint workflows

stanconia · 2024-07-09T04:00:42Z

/lgtm

@tokoko please approve lint workflows

@tokoko please can you assist with a merge

tokoko · 2024-07-09T05:04:48Z

@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 · 2024-08-27T03:38:48Z

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 · 2024-08-27T05:37:11Z

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

sdk/python/feast/infra/registry/caching_registry.py Outdated Show resolved Hide resolved

Only run async when ttl > 0

da82ba0

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

tokoko reviewed Jun 12, 2024

View reviewed changes

sdk/python/feast/infra/registry/caching_registry.py Outdated Show resolved Hide resolved

tokoko reviewed Jun 12, 2024

View reviewed changes

sdk/python/feast/infra/registry/caching_registry.py Outdated Show resolved Hide resolved

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

sdk/python/feast/repo_config.py Show resolved Hide resolved

EXPEbdodla reviewed Jun 14, 2024

View reviewed changes

sdk/python/feast/infra/registry/caching_registry.py Show resolved Hide resolved

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
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Async refresh to Sql Registry #4251

feat: Add Async refresh to Sql Registry #4251

stanconia commented Jun 3, 2024 •

edited

Loading

stanconia commented Jun 4, 2024

tokoko commented Jun 4, 2024

stanconia commented Jun 5, 2024 •

edited

Loading

tokoko commented Jun 6, 2024

stanconia commented Jun 6, 2024

tokoko commented Jun 8, 2024

stanconia commented Jun 10, 2024

tokoko commented Jun 10, 2024

stanconia commented Jun 11, 2024

stanconia commented Jun 17, 2024

tokoko commented Jun 20, 2024

stanconia commented Jun 21, 2024

stanconia commented Jun 25, 2024

tokoko commented Jun 25, 2024

HaoXuAI commented Jul 1, 2024

stanconia commented Jul 2, 2024

HaoXuAI left a comment

stanconia commented Jul 2, 2024

tokoko left a comment

stanconia commented Jul 3, 2024

franciscojavierarceo commented Jul 4, 2024

tokoko commented Jul 4, 2024

stanconia commented Jul 8, 2024

stanconia commented Jul 9, 2024

tokoko commented Jul 9, 2024

franciscojavierarceo commented Aug 27, 2024

stanconia commented Aug 27, 2024 via email

feat: Add Async refresh to Sql Registry #4251

feat: Add Async refresh to Sql Registry #4251

Conversation

stanconia commented Jun 3, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

stanconia commented Jun 4, 2024

tokoko commented Jun 4, 2024

stanconia commented Jun 5, 2024 • edited Loading

tokoko commented Jun 6, 2024

stanconia commented Jun 6, 2024

tokoko commented Jun 8, 2024

stanconia commented Jun 10, 2024

tokoko commented Jun 10, 2024

stanconia commented Jun 11, 2024

stanconia commented Jun 17, 2024

tokoko commented Jun 20, 2024

stanconia commented Jun 21, 2024

stanconia commented Jun 25, 2024

tokoko commented Jun 25, 2024

HaoXuAI commented Jul 1, 2024

stanconia commented Jul 2, 2024

HaoXuAI left a comment

Choose a reason for hiding this comment

stanconia commented Jul 2, 2024

tokoko left a comment

Choose a reason for hiding this comment

stanconia commented Jul 3, 2024

franciscojavierarceo commented Jul 4, 2024

tokoko commented Jul 4, 2024

stanconia commented Jul 8, 2024

stanconia commented Jul 9, 2024

tokoko commented Jul 9, 2024

franciscojavierarceo commented Aug 27, 2024

stanconia commented Aug 27, 2024 via email

stanconia commented Jun 3, 2024 •

edited

Loading

stanconia commented Jun 5, 2024 •

edited

Loading