Fix Online Serving unable to retrieve feature data after Feature Set update. #908

mrzzy · 2020-07-30T07:05:26Z

Problem

Feast Online Serving will throw an when the user attempts Feature Retrieval in the following scenario:

user applies feature set.
user ingests feature data for that feature set.
user updates to feature set to add or archive features.
user is unable to retrieve Feature Data from Online Serving and gets the following error:

*status.statusError: rpc error: code = DataLoss desc = Failed to decode FeatureRow from bytes retrieved from redis: Possible data corruption

What this PR does / why we need it:
Update Ingestion's RedisCustomIO to encode feature rows by setting field name to a hash of the actual name.

Used hash to reduce the increase in storage used when storing encoded feature rows in redis.

This changes the encoding of Feature Rows stored in Redis.

Update Online Serving's FeatureRowDecoder to support decoding Feature Rows by name hash.

Missing fields in encoded Feature Row would be decoded in as empty values.
Extra Fields in encoded Feature Row would be omitted from the decoded feature row.

FeatureRowDecoder will continue to support decoding existing Feature Row in existing encoding stored in Redis.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Encoding of Feature Rows stored in Redis hash changed:
- Fields in encoded Feature Row are now set to a hash of the field's actual name.

...nnectors/redis/src/main/java/feast/storage/connectors/redis/retriever/FeatureRowDecoder.java

woop · 2020-07-30T08:09:54Z

...nnectors/redis/src/main/java/feast/storage/connectors/redis/retriever/FeatureRowDecoder.java

+              .map(
+                  name -> {
+                    String nameHash =
+                        Hashing.murmur3_32().hashString(name, StandardCharsets.UTF_8).toString();


What is the length of the hash string? Just want to make sure its as small as possible.

8 characters.

Assuming that the fields stored in the feature row are float values (32 bit, 4 bytes), this would mean a ~3x increase in space consumption.
@woop @pyalex @khorshuheng

Not all fields that are stored are float, there're a lot strings as well and int64. So everything is not so bad

But I guess that we can safely cut hash string to 4-5 chars

tests/e2e/redis/basic-ingest-redis-serving.py

feast-ci-bot · 2020-08-01T04:41:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrzzy, 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:

~~OWNERS~~ [pyalex]

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

…sh of field.

This reverts commit aecc9a6e9a70be3fd84d04f81442b518be01a4c6.

… be retrieved

pyalex · 2020-08-01T05:45:25Z

/test test-end-to-end-batch-dataflow

pyalex · 2020-08-01T05:53:35Z

/lgtm

…update. (#908) * Update RedisCustomIO to write FeatureRows with field's name set to hash of field. * Update FeatureRowDecoder to decode by name hash instead of order * Bump pytest order numbers by 2 to make space for new tests * Revert "Bump pytest order numbers by 2 to make space for new tests" This reverts commit aecc9a6e9a70be3fd84d04f81442b518be01a4c6. * Added e2e to check that feature rows with missing or extra fields can be retrieved * Clarify docs about Feature Row v1 encoding and Feature Row v2 encoding * Fix python lint * Update FeatureRowDecoder's isEncodedV2 check to use anyMatch() * Make missing field/extra field e2e tests independent of other tests. * Update FeatureRowDecoder if/else statement into 2 ifs * Fix python and java lint * Fix java unit test failures * Fix ImportJobTest java unit test * Sync github workflows with master * Sync .github folder with master for fix * Replace v1/v2 encoding with v1/v2 decoder in docs

mrzzy requested review from davidheryanto, khorshuheng, pyalex, woop and zhilingc as code owners July 30, 2020 07:05

feast-ci-bot added needs-kind size/L labels Jul 30, 2020

woop reviewed Jul 30, 2020

View reviewed changes

...nnectors/redis/src/main/java/feast/storage/connectors/redis/retriever/FeatureRowDecoder.java Outdated Show resolved Hide resolved

woop reviewed Jul 30, 2020

View reviewed changes

...nnectors/redis/src/main/java/feast/storage/connectors/redis/retriever/FeatureRowDecoder.java Outdated Show resolved Hide resolved

woop reviewed Jul 30, 2020

View reviewed changes

...nnectors/redis/src/main/java/feast/storage/connectors/redis/retriever/FeatureRowDecoder.java Outdated Show resolved Hide resolved

woop reviewed Jul 30, 2020

View reviewed changes

tests/e2e/redis/basic-ingest-redis-serving.py Outdated Show resolved Hide resolved

feast-ci-bot added size/XL size/L and removed size/L size/XL labels Jul 30, 2020

pyalex approved these changes Aug 1, 2020

View reviewed changes

feast-ci-bot added the approved label Aug 1, 2020

mrzzy added 7 commits August 1, 2020 12:53

Update RedisCustomIO to write FeatureRows with field's name set to ha…

3819c07

…sh of field.

Update FeatureRowDecoder to decode by name hash instead of order

0ba8fce

Bump pytest order numbers by 2 to make space for new tests

4803ea5

Revert "Bump pytest order numbers by 2 to make space for new tests"

feb2438

This reverts commit aecc9a6e9a70be3fd84d04f81442b518be01a4c6.

Added e2e to check that feature rows with missing or extra fields can…

3491a3a

… be retrieved

Clarify docs about Feature Row v1 encoding and Feature Row v2 encoding

d2dcfb2

Fix python lint

049b73f

mrzzy added 9 commits August 1, 2020 12:53

Update FeatureRowDecoder's isEncodedV2 check to use anyMatch()

1d97462

Make missing field/extra field e2e tests independent of other tests.

5eb13db

Update FeatureRowDecoder if/else statement into 2 ifs

80f5b85

Fix python and java lint

dbd0744

Fix java unit test failures

25af940

Fix ImportJobTest java unit test

3e1a4d0

Sync github workflows with master

506f669

Sync .github folder with master for fix

c122768

Replace v1/v2 encoding with v1/v2 decoder in docs

e11222e

feast-ci-bot added size/XL and removed size/L labels Aug 1, 2020

feast-ci-bot assigned pyalex Aug 1, 2020

feast-ci-bot added the lgtm label Aug 1, 2020

pyalex added the kind/bug label Aug 1, 2020

feast-ci-bot removed the needs-kind label Aug 1, 2020

feast-ci-bot merged commit 6803457 into feast-dev:master Aug 1, 2020

mrzzy mentioned this pull request Aug 2, 2020

Backports for v0.6.2 #918

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Online Serving unable to retrieve feature data after Feature Set update. #908

Fix Online Serving unable to retrieve feature data after Feature Set update. #908

mrzzy commented Jul 30, 2020

woop Jul 30, 2020 •

edited

Loading

mrzzy Jul 30, 2020

mrzzy Jul 31, 2020

pyalex Aug 1, 2020 •

edited

Loading

pyalex Aug 1, 2020

feast-ci-bot commented Aug 1, 2020

pyalex commented Aug 1, 2020

pyalex commented Aug 1, 2020

Fix Online Serving unable to retrieve feature data after Feature Set update. #908

Fix Online Serving unable to retrieve feature data after Feature Set update. #908

Conversation

mrzzy commented Jul 30, 2020

Problem

woop Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

mrzzy Jul 30, 2020

Choose a reason for hiding this comment

mrzzy Jul 31, 2020

Choose a reason for hiding this comment

pyalex Aug 1, 2020 • edited Loading

Choose a reason for hiding this comment

pyalex Aug 1, 2020

Choose a reason for hiding this comment

feast-ci-bot commented Aug 1, 2020

pyalex commented Aug 1, 2020

pyalex commented Aug 1, 2020

woop Jul 30, 2020 •

edited

Loading

pyalex Aug 1, 2020 •

edited

Loading