Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Field Status metadata to Online Serving (#658)
* Update ServingService protobuf with additions to support online metadata retrieval. Additions are outlined in detail in this RFC: https://docs.google.com/document/d/1VQngwBcx-yWgGpAbsFVdth9GnjL8q-ZgUNBGv57R0Fk/ * Change error thrown by sendMultiGet() to Unknown as it describes the fault more accurately * Return boolean primitive instead of Boolean object type in FeatureRowDecoder This removes the unneeded possiblity of a null being returning from the the decoder. * Config sendAndProcessMultiGet() to throw an exception on feature row decoding failure. Instead of the current behaviour to silently fail with and return an empty feature row. * Refactor sendMultiGetAndProcess() to getFeaturesForFeatureSet(). Rename as the original name described the implementation instead what the goal of the method is. Construct and pass the decoder outside method in getFeaturesForFeatureSet() instead of inside method in sendMultiGetAndProcess() as constructing it inside added uncessary arguments and code (ie getting the string ref out a redis key seemed out of place.) Changed getFeaturesForFeatureSet() to return null instead of a nullFeatureRow. when retrieving data from redis using sendMultiGet() using a key missing in Redis. This makes the missing key condition easier to detect. Removed unncessary arguments passed to sendMultiGetAndProcess() - featureReference used to contruct the nullFeatureRow - featureSetSpec used to contruct the decoder. * Update ResponseJSONMapper to use new GetOnlineFeatures protobuf * Remove references to FieldValues in OnlineServingService & Tests * Update java sdk's FieldClient to use the new GetOnlineFeatures protobuf * Update OnlineRetriever to return null on missing retrieval (such as missing key for redis). * Refactor out unnecessary nesting of returned list in OnlineRetriever's getOnlineFeatures() This refactor is done to improve code readablity as it is difficult to conceptualise what each nesting of the list corresponds to: - developers have to keep in mind that the outer nest corresponds to faetures requests and the inner nesting corresponds to entity rows. The nesting is unnecessary as: - inside RedisOnlineRetriever we have a loop over feature set requests. - outside in OnlineServingService where this is used, we also loop over feature set requests. * Add online feature metadata to OnlineServingService's getOnlineFeatures() Updated getOnlineFeatures() to return metadata on each response Field. - Added sanity check with exception to ensure that the feature rows retrieved from the OnlineRetriever matches the no. of entity rows pass to it. Refactor code out of getOnlineFeatures() to smaller methods to reduce method size and improve readabilty: - added unpackFields(entityRow) to unpack EntityRows into response Fields - added unpackFields(featureRow) to unpack FeatureRows into response Fields - added attachMetadata(field) to add metadata to response Fields. - moved loop over feature references into populateStaleKeyCountMetrics() * Update OnlineServingServiceTest to check that online metadata is set correctly. * Added online metadata support java sdk's FeastClient. * Changed go sdk's Row to store a map of fields instead of just Values * Update response.go to use updated Row and take into account FieldStatus * Added Field() to go sdk's types.go to construct fields from a Value. * Update go sdk's request.go to use updated Row and Add GetOnlineFeatures flags Added missing boolean flags for GetOnlineFeatures's IncludeMetadataInResponse and OmitEntitiesInResponse flags. * Update go protobuf generated code for updated protobuf defintion * Update python sdk's client to document new fieldstatus metadata, added flags Added missing boolean flags for GetOnlineFeaturesRequest's omit_entities_in_response & include_metadata_in_response flags. Update unit tests with changes to GetOnlineFeaturesResponse protobuff * Fixed issue where getOnlineFeatures() returned Record in a non deterministic order. Enforce that getOnlineFeatures() returns Records in the same order as the user provides the EntityRow * Update e2e ingestion tests to support new get_online_features api and check for metadata * Fixed NullPointerException when feature row is null. * Fixed issue in which unpackFields(entityRow) was not responding to includeMetadata flag * Check FieldStatus in end to end tests to wait for ingestion of values to complete. * Moved test checks out of polling loop in e2e online ingestion tests * Apply spotless java formatting * Fixed misplaced break in end to end tests * Fixed typos in e2e ingestion tests. * Apply changes on RedisOnlineRetriever to RedisClusterOnlineRetriever Changes * Removed unnessary nesting of list in FeaturRows returned * Refactored sendMultiGetAndProcess() to getOnlineFeaturesForFeatureSet() Necessary due to code duplciation between RedisOnlineRetriever and RedisOnlineRetriever. * Rename getFeaturesForFeatureSet() to getFeaturesFromRedis() for better sementics * Update Serving application.yml to include example on how to config RedisCluster * Correct missing include_meta flag in e2e tests get_online_features() calls * Correct minor comment typo in serving's application.yml * Correct another minor typo in e2e test: missing .status * Fix Feast core and serving compilation/test failures after rebase * Fix go SDK tests after rebase * Update ServingService protobuf to include metadata without breaking changes. * Revert Feast Serving's ServingService & ResponseJSONMapper to FieldValues * Revert serving's OnlineServingService to backwards compatible FieldValues API * Revert Java SDK's FeastClient to backwards compatible FieldStatus API. * Revert Go SDK's client.go to backwards compatible FieldStatus API. * Fix Java SDK unit tests * Added .Statuses() method to GO Sdk's OnlineFeaturesResponse to get metadata * Fixed Java SDK not stripping project from field values and statuses * Fixed Go SDK's client.go not stripping project from field values and statuses * Fixed Go SDK's Unit tests * Revert Python SDK to backward compatible FieldValues API & fix tests * Move hardcoded constants from job.py to constants.py in Python SDK to consolidate constants * Added wait backoff implementation in Python SDK: wait_retry_backoff() * Update Python SDK's job.py to use shared backoff wait implemetation * Clarify that maximum range in ServingService proto to max age * Fixed online serving e2e tests * Fix python lint. * Collect entity refs in Python SDK as a single line. * Use guard style if statements in Go Sdk's types.go * Make strip field values part of Python SDK's get_online_features() more functional * Fix go unit tests * Remove include_metadata_in_response flag from ServingService proto as its redundant. * Readd missing span log call to log retrieved feature rows. * Use more modern Streams.zip() to iterate entity rows and feature rows in OnlineServingService * Change utility methods in OnlineServingService to static. * Use "outside max age" instead "stale" term in OnlineServingService as "stale" is a overloaded term. * Refactor serving's OnlineRetriever to return a empty Optional<FeatureRow> 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 * Fixed typo OnlineServingService's logFeatureRowsTrace() Co-authored-by: Zhu Zhanyan <zhu.zhanyan@gojek.com>
- Loading branch information
