-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Java feature server] Converge ServingService API to make Python and Java feature servers consistent #2166
[Java feature server] Converge ServingService API to make Python and Java feature servers consistent #2166
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2166 +/- ##
==========================================
+ Coverage 84.57% 84.58% +0.01%
==========================================
Files 102 102
Lines 8195 8201 +6
==========================================
+ Hits 6931 6937 +6
Misses 1264 1264
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
de882a7
to
f3ea70c
Compare
Benchmarks were also added in Java integration tests
Full FS == feature service with 250 features |
f3ea70c
to
e92609c
Compare
java/serving/src/main/java/feast/serving/registry/RegistryRepository.java
Outdated
Show resolved
Hide resolved
java/serving/src/main/java/feast/serving/service/OnlineServingServiceV2.java
Outdated
Show resolved
Hide resolved
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
b124a10
to
a795c49
Compare
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
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.
also change the release notes to say that the java FS supports feature services now?
java/serving/src/main/java/feast/serving/service/OnlineServingServiceV2.java
Outdated
Show resolved
Hide resolved
@@ -94,6 +87,11 @@ message GetOnlineFeaturesRequest { | |||
// A map of entity name -> list of values | |||
map<string, feast.types.RepeatedValue> entities = 3; | |||
bool full_feature_names = 4; | |||
|
|||
// Context for OnDemand Feature Transformation |
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.
to clarify, this is the "request_data" that we had before right?
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.
To my knowledge, there was no "request_data" in protos or Python API. Not sure what you're referring to
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.
This was used for the on demand feature views. You had touched some of this in the transformation logic (it used to be passed in as part of the entity row). I think this is the same.
Can you also add a new issue to migrate the python logic for this too?
java/serving/src/main/java/feast/serving/service/OnlineServingServiceV2.java
Show resolved
Hide resolved
Signed-off-by: pyalex <moskalenko.alexey@gmail.com>
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adchia, pyalex The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Currently, Java Feature Server and Python Feature Server provide two different client APIs. This PR aimed to fix that.
GetOnlineFeatureRequest
(that was created for and currently used by Python FS) as input inGetOnlineFeatures
method. That also means that Java FS will support requests by feature service.GetOnlineFeatureResponseV2
is proposed as optimized version ofGetOnlineFeatureResponse
and was designed symmetrically toGetOnlineFeatureRequest
. According to my evaluation serialization/deserialization of the new message is 3x times faster (on 100 rows). That difference increases with a growing number of rows and features. New message also optimized for compaction (message size), which will lead to faster network transmission.What was updated:
What's not covered in this PR:
Some clean up:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: