Skip to content

Local feature server implementation (HTTP endpoint)#1780

Merged
feast-ci-bot merged 3 commits into
feast-dev:masterfrom
tsotnet:tsotne/feature_server
Aug 21, 2021
Merged

Local feature server implementation (HTTP endpoint)#1780
feast-ci-bot merged 3 commits into
feast-dev:masterfrom
tsotnet:tsotne/feature_server

Conversation

@tsotnet
Copy link
Copy Markdown
Collaborator

@tsotnet tsotnet commented Aug 13, 2021 •
edited
Loading

Signed-off-by: Tsotne Tabidze tsotne@tecton.ai

What this PR does / why we need it: Implements the local HTTP feature consumption server.

RFCs for context:

From one terminal tab we can run feast serve:

$ feast serve
INFO:     Started server process [13615]
08/12/2021 09:29:36 PM INFO:Started server process [13615]
INFO:     Waiting for application startup.
08/12/2021 09:29:36 PM INFO:Waiting for application startup.
INFO:     Application startup complete.
08/12/2021 09:29:36 PM INFO:Application startup complete.
INFO:     Uvicorn running on http://127.0.0.1:6566 (Press CTRL+C to quit)
08/12/2021 09:29:36 PM INFO:Uvicorn running on http://127.0.0.1:6566 (Press CTRL+C to quit)
INFO:     127.0.0.1:52654 - "GET /get-online-features/ HTTP/1.1" 200 OK
INFO:     127.0.0.1:52660 - "GET /get-online-features/ HTTP/1.1" 200 OK

From another tab we can run cURL requests:

$ curl -X GET \
    "http://127.0.0.1:6566/get-online-features/" \
    -H "Content-type: application/json" \
    -H "Accept: application/json" \
    -d '{
        "features": [
            "driver_hourly_stats:conv_rate",
            "driver_hourly_stats:acc_rate",
            "driver_hourly_stats:avg_daily_trips"
        ],
        "entities": {
            "driver_id": [1001, 1002, 1003]
        },
        "full_feature_names": true
    }' | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1345  100  1054  100   291   3685   1017 --:--:-- --:--:-- --:--:--  4702
{
  "field_values": [
    {
      "fields": {
        "driver_hourly_stats__conv_rate": 0.5315734148025513,
        "driver_id": 1001,
        "driver_hourly_stats__acc_rate": 0.5842991471290588,
        "driver_hourly_stats__avg_daily_trips": 714
      },
      "statuses": {
        "driver_hourly_stats__conv_rate": "PRESENT",
        "driver_hourly_stats__acc_rate": "PRESENT",
        "driver_hourly_stats__avg_daily_trips": "PRESENT",
        "driver_id": "PRESENT"
      }
    },
    {
      "fields": {
        "driver_id": 1002,
        "driver_hourly_stats__avg_daily_trips": 133,
        "driver_hourly_stats__conv_rate": 0.23269200325012207,
        "driver_hourly_stats__acc_rate": 0.5605814456939697
      },
      "statuses": {
        "driver_hourly_stats__avg_daily_trips": "PRESENT",
        "driver_hourly_stats__acc_rate": "PRESENT",
        "driver_id": "PRESENT",
        "driver_hourly_stats__conv_rate": "PRESENT"
      }
    },
    {
      "fields": {
        "driver_hourly_stats__conv_rate": 0.7948746085166931,
        "driver_hourly_stats__avg_daily_trips": 506,
        "driver_id": 1003,
        "driver_hourly_stats__acc_rate": 0.9278625845909119
      },
      "statuses": {
        "driver_hourly_stats__acc_rate": "PRESENT",
        "driver_hourly_stats__avg_daily_trips": "PRESENT",
        "driver_hourly_stats__conv_rate": "PRESENT",
        "driver_id": "PRESENT"
      }
    }
  ]

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Implement `feast serve` CLI command that starts a local HTTP feature server

Local feature server implementation (HTTP endpoint)
dd5abb8 
Signed-off-by: Tsotne Tabidze <tsotne@tecton.ai>
@tsotnet tsotnet added kind/feature New feature or request area/serving labels Aug 13, 2021
@tsotnet tsotnet requested review from felixwang9817 and woop August 13, 2021 04:40
@tsotnet tsotnet requested review from a team and achals as code owners August 13, 2021 04:40
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 13, 2021 •
edited
Loading

Codecov Report

Merging #1780 (11acb49) into master (8ef2053) will increase coverage by 0.51%.
The diff coverage is 80.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1780      +/-   ##
==========================================
+ Coverage   84.71%   85.23%   +0.51%     
==========================================
  Files          86       91       +5     
  Lines        6308     6752     +444     
==========================================
+ Hits         5344     5755     +411     
- Misses        964      997      +33
Flag Coverage Δ
integrationtests 85.15% <80.95%> (+0.49%) ⬆️
unittests 63.96% <80.95%> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdk/python/feast/feature_server.py 26.47% <26.47%> (ø)
sdk/python/feast/cli.py 43.95% <50.00%> (+0.27%) ⬆️
sdk/python/feast/proto_json.py 95.45% <95.45%> (ø)
sdk/python/feast/feature_store.py 92.64% <100.00%> (-0.94%) ⬇️
sdk/python/feast/type_map.py 50.83% <100.00%> (ø)
sdk/python/tests/unit/test_proto_json.py 100.00% <100.00%> (ø)
...ion/feature_repos/universal/data_source_creator.py 77.77% <0.00%> (-0.80%) ⬇️
sdk/python/feast/feature_view.py 87.05% <0.00%> (-0.72%) ⬇️
sdk/python/feast/usage.py 60.95% <0.00%> (-0.59%) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ef2053...11acb49. Read the comment docs.

felixwang9817
felixwang9817 reviewed Aug 13, 2021
View reviewed changes
Comment thread sdk/python/feast/feature_store.py Outdated
felixwang9817
felixwang9817 reviewed Aug 13, 2021
View reviewed changes
Comment thread sdk/python/feast/feature_server.py
woop
woop reviewed Aug 13, 2021
View reviewed changes
Comment thread protos/feast/serving/ServingService.proto Outdated
Update the request protobufs and hack json/protobuf conversion to avo…
980024c 
…id extra structure (e.g. "int64_val") in json

Signed-off-by: Tsotne Tabidze <tsotne@tecton.ai>
@tsotnet
Copy link
Copy Markdown
Collaborator Author

tsotnet commented Aug 17, 2021

The request API is now much more compact. Here's an example JSON request:

{
        "features": [
            "driver_hourly_stats:conv_rate",
            "driver_hourly_stats:acc_rate",
            "driver_hourly_stats:avg_daily_trips"
        ],
        "entities": {
            "driver_id": [1001, 1002, 1003]
        },
        "full_feature_names": true
    }

I haven't modified the response yet. I'm thinking about making it columnar as well, and making the statuses an optional field (will need to be set manually by user get_statuses=true. @woop @felixwang9817 what do you guys think?

@woop
Copy link
Copy Markdown
Member

woop commented Aug 17, 2021

The request API is now much more compact. Here's an example JSON request:

{
        "features": [
            "driver_hourly_stats:conv_rate",
            "driver_hourly_stats:acc_rate",
            "driver_hourly_stats:avg_daily_trips"
        ],
        "entities": {
            "driver_id": [1001, 1002, 1003]
        },
        "full_feature_names": true
    }

I haven't modified the response yet. I'm thinking about making it columnar as well, and making the statuses an optional field (will need to be set manually by user get_statuses=true. @woop @felixwang9817 what do you guys think?

Agreed on the above. @tsotnet do we have an idea of the performance impact of using a columnar representation? Ultimately we'll be converting the representation into row level keys in order to do lookups. Can we do it in a way that has little overhead?

Revert update to the service
11acb49 
Signed-off-by: Tsotne Tabidze <tsotne@tecton.ai>
@tsotnet
Copy link
Copy Markdown
Collaborator Author

tsotnet commented Aug 19, 2021

Agreed on the above. @tsotnet do we have an idea of the performance impact of using a columnar representation? Ultimately we'll be converting the representation into row level keys in order to do lookups. Can we do it in a way that has little overhead?

I haven't done benchmarks, but I suspect that the savings from the network overhead (due to the smaller request size) will be more significant than columnar to row level in-memory transformation.

felixwang9817
felixwang9817 approved these changes Aug 21, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@felixwang9817 felixwang9817 left a comment

Choose a reason for hiding this comment

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

/lgtm

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Aug 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: felixwang9817, tsotnet

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit 745a1b4 into feast-dev:master Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop left review comments

@felixwang9817 felixwang9817 felixwang9817 approved these changes

@achals achals Awaiting requested review from achals

Assignees

@felixwang9817 felixwang9817

Labels

approved area/serving kind/feature New feature or request lgtm release-note size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tsotnet @codecov-commenter @woop @feast-ci-bot @felixwang9817