Add Field Status metadata to Online Serving #658

mrzzy · 2020-04-27T08:48:58Z

What this PR does / why we need it:
Adds field status metadata to Online Serving so that users can debug Serving returning missing values.

Field status metadata returned (FieldStatus) classifies the missing values into 3 possible cases:

NULL_VALUE - the value is missing because the user did not specify a value on ingestion.
NOT_FOUND - the value is missing because it could not be retrieved from the online store (ie feature data key not found Redis store.)
OUTSIDE_MAX_AGE - the value is missing because it was purposely discarded as the time since the value was ingested and retrieval has exceeded the FeatureSet's max age.

Updates SDKs (Go, Java, Python) to support field status metadata.

Which issue(s) this PR fixes:

Fixes #278

Does this PR introduce a user-facing change?:

Add Field Status metadata to Online Serving:
* changes to ServingService Protobuf:
  - Added statuses map to FieldValues.
  - Added FieldStatus enum to define Field Status states.
* Updated SDKs to support Field Status Metadata.

mrzzy · 2020-04-28T10:08:07Z

/test test-serving

mrzzy · 2020-04-28T10:08:41Z

/test test-python-sdk

mrzzy · 2020-04-28T10:08:49Z

/test test-golang-sdk

mrzzy · 2020-04-28T10:09:11Z

/test test-end-to-end

mrzzy · 2020-04-28T10:09:33Z

/test test-end-to-end-redis-cluster

ches · 2020-04-29T05:17:42Z

Just a very, um, meta thing—if I saw this changelog and #536 in the same release notes I would think they're related. Should we find some disambiguating terminology? Maybe "registry metadata" and "serving metadata"?

ches · 2020-04-29T05:19:23Z

I guess "feature (set) labels" could already suffice unambiguously for what I called "registry metadata". The feature in this PR being called "feature metadata" is problematic though, I think.

woop · 2020-04-29T14:23:19Z

Just a very, um, meta thing—if I saw this changelog and #536 in the same release notes I would think they're related. Should we find some disambiguating terminology? Maybe "registry metadata" and "serving metadata"?

Yea I agree :)

Actually the Feast project has a long history of abusing the term metadata. It's just such a convenient word. It's everything Util, Spec, and Info aspire to be, and more.

How about

Feature status metrics
Feature status metadata
Feature value statuses <- my preference.

ches · 2020-04-29T18:05:22Z

It's everything Util, Spec, and Info aspire to be, and more.

I might steal that.

I think "feature value statuses" is apt.

ches · 2020-04-29T18:21:08Z

Can we introduce this change/functionality in an evolutionary-friendly way? Like introduce it as a new RPC and new messages, with the old ones remaining supported, possibly going through a deprecation period before Feast 1.0?

Breaking the contract is extremely disruptive here. Upgrading the Serving API will break existing online clients in production, they will all have to upgrade their SDK, and it's not possible to roll out a forward-compatible SDK upgrade to fleets in advance. This breaks all the rules of API evolution.

woop · 2020-04-30T01:11:40Z

@mrzzy any objections to feature value statuses?

Breaking the contract is extremely disruptive here. Upgrading the Serving API will break existing online clients in production, they will all have to upgrade their SDK, and it's not possible to roll out a forward-compatible SDK upgrade to fleets in advance. This breaks all the rules of API evolution.

We can. Some ideas on gRPC versioning from Microsoft.

We could add a new RPC with V2?

mrzzy · 2020-04-30T02:11:41Z

I renaming "Feature Metadata" to "Field Status" in the PR name instead of "Feature Status" as it also returns entity statuses in addition to feature statuses.

zhilingc · 2020-04-30T04:44:13Z

The golang code has to be versioned too, since version pinning is a non-goal of go modules :/

ches · 2020-04-30T08:05:37Z

This is a good example I think in favor of SDKs providing higher-level API wrapping protobuf types and request/response objects—this Java API was seamlessly upgraded in the PR, and the same could apply to using a new RPC underneath and phasing out the old one:

public List<Row> getOnlineFeatures(List<String> features, List<Row> rows, String defaultProject)

while this is very susceptible to source-incompatible breakage, at least if user code touches the (public) underlying RawResponse:

func (fc *GrpcClient) GetOnlineFeatures(ctx context.Context, req *OnlineFeaturesRequest) (
	*OnlineFeaturesResponse, error) {

zhilingc · 2020-04-30T08:19:23Z

The golang SDK follows a similar pattern, but unfortunately for golang the proto-generated code is persisted (and versioned) together with the sdk code :(

type Client interface {
	GetOnlineFeatures(ctx context.Context, req *OnlineFeaturesRequest) (*OnlineFeaturesResponse, error)
}

…statuses

… consolidate constants

…re functional

… its redundant.

… in OnlineServingService

… "stale" is a overloaded term.

…Row> instead of returning null. This allows the code to more semenatic and readable compared to dealing with nullable FeatureRow values

woop · 2020-06-19T05:45:41Z

/lgtm

feast-ci-bot · 2020-06-19T05:45:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrzzy, woop

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:

~~OWNERS~~ [woop]

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

woop · 2020-06-19T06:14:26Z

/test test-end-to-end-batch

feast-ci-bot added do-not-merge/work-in-progress size/XXL labels Apr 27, 2020

mrzzy changed the title ~~WIP: Add feature metadata to Online Serving~~ Add feature metadata to Online Serving Apr 28, 2020

mrzzy marked this pull request as ready for review April 28, 2020 10:22

mrzzy requested review from davidheryanto, khorshuheng, pradithya, woop and zhilingc as code owners April 28, 2020 10:22

feast-ci-bot removed the do-not-merge/work-in-progress label Apr 28, 2020

woop mentioned this pull request Apr 29, 2020

Feast 0.5 release #527

Closed

woop added this to the v0.6.0 milestone Apr 29, 2020

ches added the compat/breaking Breaking user-facing change label Apr 29, 2020

mrzzy changed the title ~~Add feature metadata to Online Serving~~ Add Field Status metadata to Online Serving Apr 30, 2020

woop added the kind/feature New feature or request label May 3, 2020

mrzzy added 22 commits June 18, 2020 21:51

Fixed Java SDK not stripping project from field values and statuses

5141a1e

Fixed Go SDK's client.go not stripping project from field values and …

059ea9f

…statuses

Fixed Go SDK's Unit tests

8d5d031

Revert Python SDK to backward compatible FieldValues API & fix tests

bf9c11b

Move hardcoded constants from job.py to constants.py in Python SDK to…

87249c8

… consolidate constants

Added wait backoff implementation in Python SDK: wait_retry_backoff()

ccd632d

Update Python SDK's job.py to use shared backoff wait implemetation

4580e04

Clarify that maximum range in ServingService proto to max age

95f5f6b

Fixed online serving e2e tests

3bf17f2

Fix python lint.

bd54fc7

Collect entity refs in Python SDK as a single line.

64f59ac

Use guard style if statements in Go Sdk's types.go

f1e52fc

Make strip field values part of Python SDK's get_online_features() mo…

9958e7e

…re functional

Fix go unit tests

6de8267

Remove include_metadata_in_response flag from ServingService proto as…

7952d98

… its redundant.

Readd missing span log call to log retrieved feature rows.

32cccaa

Use more modern Streams.zip() to iterate entity rows and feature rows…

c05614d

… in OnlineServingService

Change utility methods in OnlineServingService to static.

81422cc

Use "outside max age" instead "stale" term in OnlineServingService as…

8df3e57

… "stale" is a overloaded term.

Refactor serving's OnlineRetriever to return a empty Optional<Feature…

a03a1a9

…Row> instead of returning null. This allows the code to more semenatic and readable compared to dealing with nullable FeatureRow values

Fixed typo in basic redis e2e tests

1e97264

Fixed typo OnlineServingService's logFeatureRowsTrace()

d4ef594

feast-ci-bot assigned woop Jun 19, 2020

feast-ci-bot added the lgtm label Jun 19, 2020

woop approved these changes Jun 19, 2020

View reviewed changes

feast-ci-bot added the approved label Jun 19, 2020

feast-ci-bot merged commit eebf458 into feast-dev:master Jun 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Field Status metadata to Online Serving #658

Add Field Status metadata to Online Serving #658

mrzzy commented Apr 27, 2020 •

edited

Loading

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

ches commented Apr 29, 2020

ches commented Apr 29, 2020 •

edited

Loading

woop commented Apr 29, 2020 •

edited

Loading

ches commented Apr 29, 2020

ches commented Apr 29, 2020

woop commented Apr 30, 2020

mrzzy commented Apr 30, 2020 •

edited

Loading

zhilingc commented Apr 30, 2020

ches commented Apr 30, 2020

zhilingc commented Apr 30, 2020 •

edited

Loading

woop commented Jun 19, 2020

feast-ci-bot commented Jun 19, 2020

woop commented Jun 19, 2020

Add Field Status metadata to Online Serving #658

Add Field Status metadata to Online Serving #658

Conversation

mrzzy commented Apr 27, 2020 • edited Loading

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

mrzzy commented Apr 28, 2020

ches commented Apr 29, 2020

ches commented Apr 29, 2020 • edited Loading

woop commented Apr 29, 2020 • edited Loading

ches commented Apr 29, 2020

ches commented Apr 29, 2020

woop commented Apr 30, 2020

mrzzy commented Apr 30, 2020 • edited Loading

zhilingc commented Apr 30, 2020

ches commented Apr 30, 2020

zhilingc commented Apr 30, 2020 • edited Loading

woop commented Jun 19, 2020

feast-ci-bot commented Jun 19, 2020

woop commented Jun 19, 2020

mrzzy commented Apr 27, 2020 •

edited

Loading

ches commented Apr 29, 2020 •

edited

Loading

woop commented Apr 29, 2020 •

edited

Loading

mrzzy commented Apr 30, 2020 •

edited

Loading

zhilingc commented Apr 30, 2020 •

edited

Loading