feat: Remote offline Store by redhatHameed · Pull Request #4262 · feast-dev/feast

redhatHameed · 2024-06-07T20:47:50Z

What this PR does / why we need it:

This PR added feature for remote offline store contain below changes.

Added an Offline Server using Arrow Flight:

Implement the remote OfflineServer similar to the one developed for the Remote Registry or Feature Server but using Arrow Flight Server instead of pure gRPC.
Added the command into cli for starting the offline server using the feast serve_offline command.
Implemented the the OfflineStore interface methods as arrow flight server methods.
Added unit test and integration test functions for the OfflineServer class.

Added an Offline Remote Client:

Implement the offline remote client using arrow flight remote client approach, which supports the type: remote configuration in the offline store configuration for example as seen below.

    type: remote 
    host: mystore.feast.svc.cluster.local
     port: 8815

Develop the RemoteOfflineStore class, which implements the existing OfflineStore interface in Feast. Utilize the get_historical_features method and pass parameters such as entity definition, feature views, offline store config to interact with the Arrow server functions using the Arrow Flight client.
Client implemented the other methods like pull_latest_from_table_or_query, write_logged_features , offline_write_batch
added unit test function to test the remote client.

Updated Helm Chart for Deploying as a service on Kubernetes:

updated chart deployment to install offline server by using feast_mode property based on the deployment choice user can set the property. The online mode is the default and maintains backward compatibility with previous Feast Feature Server implementations.

Helm install examples:

helm install feast-feature-server feast-charts/feast-feature-server --set feature_store_yaml_base64=$(base64 > feature_store.yaml)
helm install feast-offline-server feast-charts/feast-feature-server --set feast_mode=offline  --set feature_store_yaml_base64=$(base64 > feature_store.yaml)
helm install feast-ui-server feast-charts/feast-feature-server --set feast_mode=ui  --set feature_store_yaml_base64=$(base64 > feature_store.yaml)
helm install feast-registry-server feast-charts/feast-feature-server --set feast_mode=registry  --set feature_store_yaml_base64=$(base64 > feature_store.yaml)

Added documentation for remote offline support

Added example for remote offline

Which issue(s) this PR fixes:

Fixes #4032

Fixes

sdk/python/feast/offline_server.py

sdk/python/feast/infra/offline_stores/remote.py

…ight server and client Signed-off-by: Abdul Hameed <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

…ed _make_flight_info Signed-off-by: Abdul Hameed <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

… an environment variable Signed-off-by: Theodor Mihalache <[email protected]> Signed-off-by: Abdul Hameed <[email protected]>

use feature_view_names to transfer feature views and remove dummies Signed-off-by: Abdul Hameed <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

…ight server and client Signed-off-by: Abdul Hameed <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

…), offline, ui and registry Signed-off-by: Abdul Hameed <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

tokoko · 2024-06-12T03:47:38Z

@redhatHameed just curious, are the tests failing only in gh workflows? Do they pass locally?

dmartinol · 2024-06-12T10:41:55Z

Just a note, if we decide in the future to use this server for remote materialization engine as well, the name might no longer be a good fit. Just pointing it out.. I don't have any better ideas.

Maybe it's too early to kickoff a thread, but in that case, shouldn't we have a separate materialization server with its own codebase? (if you have an issue we can move the discussion there)

tokoko · 2024-06-12T10:49:17Z

Maybe it's too early to kickoff a thread, but in that case, shouldn't we have a separate materialization server with its own codebase? (if you have an issue we can move the discussion there)

maybe, that would certainly be a cleaner approach, but imho wouldn't really serve any other practical purpose. I'll open a ticket and we probably should pick this up later once remote offline is a bit more fleshed out.

dmartinol · 2024-06-12T10:50:31Z

@redhatHameed just curious, are the tests failing only in gh workflows? Do they pass locally?

@tokoko Yes, apart some of them that fail locally. Do you have any idea why from the GH action execution we don't even see the log of the Test local integration tests step? Do you have any chance to re-run it and enable the debugging option?

tokoko · 2024-06-12T11:03:46Z

I don't have any privileges there. @franciscojavierarceo @jeremyary any ideas?

sdk/python/tests/integration/feature_repos/universal/data_sources/file.py

redhatHameed · 2024-06-12T12:54:00Z

@redhatHameed just curious, are the tests failing only in gh workflows? Do they pass locally?

Yes it does - apart from the one test case -> test_spark_materialization_consistency with error - py4j.protocol.Py4JJavaError: An error occurred while calling o114.count. but if I skip it all are passing locally see the here results

sdk/python/tests/integration/feature_repos/universal/data_sources/file.py

lokeshrangineni · 2024-06-12T20:42:57Z

sdk/python/feast/offline_server.py

+        del self.flights[key]
+        return fl.RecordBatchStream(table)
+
+    def offline_write_batch(self, command, key):


it is not a mandatory to give the types, but it makes it more readable to have types unless it is expected to have any data type.

@lokeshrangineni
You mean in the methods parameters or the return value or both?
I guess you mean for all the methods and not only this specific one?

I was specifically about method parameters. but if there is no change in functionality better to add types to return variable also.

@lokeshrangineni the change implemented under commit thanks @tmihalac addressing it

Signed-off-by: Abdul Hameed <[email protected]>

docs/SUMMARY.md

Signed-off-by: Theodor Mihalache <[email protected]>

Signed-off-by: Abdul Hameed <[email protected]>

update the document change and fix teardown function

redhatHameed · 2024-06-13T18:21:27Z

@jeremyary any thoughts I am getting incorrect credentials error one of integration test it was running fine previously ERROR sdk/python/tests/integration/offline_store/test_push_features_to_offline_store.py::test_push_features_and_read[AWS:Redshift:redis:python_fs:False] - feast.errors.RedshiftCredentialsError: Redshift API failed due to incorrect credentials ->https://github.com/feast-dev/feast/actions/runs/9504367262/job/26196850053?pr=4262

Signed-off-by: Theodor Mihalache <[email protected]>

tokoko · 2024-06-13T18:46:06Z

@redhatHameed It's a rate limit error, says An error occurred (ValidationException) when calling the DescribeTable operation: Rate exceeded if you scroll up. Redshift is being a little dramatic in the final error. A rerun should fix it.

Implemented PR change proposal

redhatHameed · 2024-06-13T19:38:14Z

@tokoko can we get your approval on this if no more comments. Also who will approve this for merging ?

tokoko

Yup, lgtm. great work, guys. @jeremyary or @franciscojavierarceo can approve and merge

# [0.39.0](v0.38.0...v0.39.0) (2024-06-18) ### Bug Fixes * Feast UI importlib change ([#4248](#4248)) ([5d486b8](5d486b8)) * Feature server no_feature_log argument error ([#4255](#4255)) ([15524ce](15524ce)) * Feature UI Server image won't start in an OpenShift cluster ([#4250](#4250)) ([4891f76](4891f76)) * Handles null values in data during GO Feature retrieval ([#4274](#4274)) ([c491e57](c491e57)) * Make Java gRPC client use timeouts as expected ([#4237](#4237)) ([f5a37c1](f5a37c1)) * Remove self assignment code line. ([#4238](#4238)) ([e514f66](e514f66)) * Set default values for feature_store.serve() function ([#4225](#4225)) ([fa74438](fa74438)) ### Features * Add online_read_async for dynamodb ([#4244](#4244)) ([b5ef384](b5ef384)) * Add the ability to list objects by `tags` ([#4246](#4246)) ([fbf92da](fbf92da)) * Added deadline to gRPC Java client ([#4217](#4217)) ([ff429c9](ff429c9)) * Adding vector search for sqlite ([#4176](#4176)) ([2478831](2478831)) * Change get_online_features signature, move online retrieval functions to utils ([#4278](#4278)) ([7287662](7287662)) * Feature/adding remote online store ([#4226](#4226)) ([9454d7c](9454d7c)) * List all feature views ([#4256](#4256)) ([36a574d](36a574d)) * Make RegistryServer writable ([#4231](#4231)) ([79e1143](79e1143)) * Remote offline Store ([#4262](#4262)) ([28a3d24](28a3d24)) * Set optional full-scan for deletion ([#4189](#4189)) ([b9cadd5](b9cadd5))

tokoko reviewed Jun 7, 2024

View reviewed changes

sdk/python/feast/offline_server.py Outdated Show resolved Hide resolved

tokoko reviewed Jun 7, 2024

View reviewed changes

sdk/python/feast/infra/offline_stores/remote.py Outdated Show resolved Hide resolved

tokoko added the ok-to-test label Jun 8, 2024

redhatHameed force-pushed the remote_offline branch 2 times, most recently from ddbc10a to 7f64708 Compare June 10, 2024 18:52

redhatHameed and others added 25 commits June 10, 2024 14:57

feat: Added offline store remote deployment functionly using arrow fl…

47a9cfb

…ight server and client Signed-off-by: Abdul Hameed <[email protected]>

Initial functional commit for remote get_historical_features

088d739

Signed-off-by: Abdul Hameed <[email protected]>

remote offline store example

241fe50

Signed-off-by: Abdul Hameed <[email protected]>

removing unneeded test code and fixinf impotrts

e276704

Signed-off-by: Abdul Hameed <[email protected]>

call do_put only once, postpone the invocation of do_put and simplifi…

cdd8a4e

…ed _make_flight_info Signed-off-by: Abdul Hameed <[email protected]>

added primitive parameters to the command descriptor

54f6061

Signed-off-by: Abdul Hameed <[email protected]>

removed redundant param

be91d23

Signed-off-by: Abdul Hameed <[email protected]>

Initial skeleton of unit test for offline server

f074044

Signed-off-by: Abdul Hameed <[email protected]>

added unit test for offline store remote client

06fe80d

Signed-off-by: Abdul Hameed <[email protected]>

testing all offlinestore APIs

49857da

Signed-off-by: Abdul Hameed <[email protected]>

integrated comments

a30c666

Signed-off-by: Abdul Hameed <[email protected]>

Updated remote offline server readme with the capability to init with…

2c8d677

… an environment variable Signed-off-by: Theodor Mihalache <[email protected]> Signed-off-by: Abdul Hameed <[email protected]>

added RemoteOfflineStoreDataSourceCreator,

1bca528

use feature_view_names to transfer feature views and remove dummies Signed-off-by: Abdul Hameed <[email protected]>

added missing CI requirement

f035430

Signed-off-by: Abdul Hameed <[email protected]>

fixed linter

632a4c0

Signed-off-by: Abdul Hameed <[email protected]>

fixed multiprocess CI requirement

94f927b

Signed-off-by: Abdul Hameed <[email protected]>

feat: Added offline store remote deployment functionly using arrow fl…

36b3479

…ight server and client Signed-off-by: Abdul Hameed <[email protected]>

fix test errors

799ae07

Signed-off-by: Abdul Hameed <[email protected]>

managing feature view aliases and restored skipped tests

3029881

Signed-off-by: Abdul Hameed <[email protected]>

fixced linter issue

f6c481b

Signed-off-by: Abdul Hameed <[email protected]>

fixed broken test

871e5d4

Signed-off-by: Abdul Hameed <[email protected]>

added supported deployment modes using helm chart for online (default…

ddc3600

…), offline, ui and registry Signed-off-by: Abdul Hameed <[email protected]>

updated the document for offline remote server

9a34251

Signed-off-by: Abdul Hameed <[email protected]>

added the document for remote offline server

efeeeae

Signed-off-by: Abdul Hameed <[email protected]>

rebase and fix conflicts

7077bee

Signed-off-by: Abdul Hameed <[email protected]>

redhatHameed force-pushed the remote_offline branch 4 times, most recently from 2eb38d1 to 2818626 Compare June 11, 2024 23:58

dmartinol reviewed Jun 12, 2024

View reviewed changes

sdk/python/tests/integration/feature_repos/universal/data_sources/file.py Outdated Show resolved Hide resolved

tokoko reviewed Jun 12, 2024

View reviewed changes

sdk/python/tests/integration/feature_repos/universal/data_sources/file.py Outdated Show resolved Hide resolved

redhatHameed force-pushed the remote_offline branch 2 times, most recently from 2e1879a to 08c2616 Compare June 12, 2024 20:07

lokeshrangineni reviewed Jun 12, 2024

View reviewed changes

fixing the integration tests

dec05c9

Signed-off-by: Abdul Hameed <[email protected]>

redhatHameed force-pushed the remote_offline branch from 08c2616 to dec05c9 Compare June 13, 2024 02:28

tokoko reviewed Jun 13, 2024

View reviewed changes

docs/SUMMARY.md Outdated Show resolved Hide resolved

tmihalac and others added 3 commits June 13, 2024 09:05

Fixed OfflineServer teardown

143ef18

Signed-off-by: Theodor Mihalache <[email protected]>

updated the document for remote offline feature server and client

bd9cd79

Signed-off-by: Abdul Hameed <[email protected]>

Merge pull request #15 from redhatHameed/doc-change

bdf0150

update the document change and fix teardown function

Implemented PR change proposal

17c7e72

Signed-off-by: Theodor Mihalache <[email protected]>

Merge pull request #16 from tmihalac/implement-remaining-offline-methods

11a3dae

Implemented PR change proposal

tokoko approved these changes Jun 13, 2024

View reviewed changes

jeremyary approved these changes Jun 13, 2024

View reviewed changes

jeremyary merged commit 28a3d24 into feast-dev:master Jun 13, 2024

Conversation

redhatHameed commented Jun 7, 2024

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Uh oh!

Uh oh!

Uh oh!

tokoko commented Jun 12, 2024

Uh oh!

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

Uh oh!

Uh oh!

tokoko commented Jun 12, 2024

Uh oh!

dmartinol commented Jun 12, 2024

Uh oh!

tokoko commented Jun 12, 2024

Uh oh!

Uh oh!

redhatHameed commented Jun 12, 2024

Uh oh!

Uh oh!

lokeshrangineni Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lokeshrangineni Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

redhatHameed Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

tokoko commented Jun 13, 2024

Uh oh!

redhatHameed commented Jun 13, 2024

Uh oh!

tokoko left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dmartinol commented Jun 12, 2024 •

edited

Loading

tmihalac Jun 13, 2024 •

edited

Loading

redhatHameed commented Jun 13, 2024 •

edited

Loading