Skip to content
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

Async job management #361

Merged
merged 12 commits into from
Dec 17, 2019
Merged

Async job management #361

merged 12 commits into from
Dec 17, 2019

Conversation

zhilingc
Copy link
Collaborator

Currently jobs are kicked off in a synchronous way when feature sets are applied. This is an issue because it is:

  • prone to race conditions which leads to failures when multiple feature sets are being applied.
  • unresponsive for the user and inefficient on feast's side, particularly when the dataflow runner is used

This PR moves all job management to a worker thread.

The goal is to:

  • Allow feature sets to be applied without blocking for a more snappy user experience
  • Allow job updates to happen in the background without the user knowing about it
  • Test and prevent any race conditions

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhilingc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zhilingc
Copy link
Collaborator Author

/hold

@woop
Copy link
Member

woop commented Dec 12, 2019

This is still a [WIP]?

@zhilingc
Copy link
Collaborator Author

Yeah. I haven't implemented safe ingestion yet (hence the e2e tests are failing)

@zhilingc
Copy link
Collaborator Author

zhilingc commented Dec 16, 2019

Pushed changes for safe ingestion: This mainly involves

  • maintaining feature set status via the list of jobs that have to run,
  • returning this status to clients via the GetFeatureSet/ListFeatureSet methods.
  • Clients poll until feast indicates that the it is safe to ingest the feature set, then ingest the data.

Unfortunately this might introduce quite a bit of lag, particularly if a user submits their feature set directly after feast starts updating its jobs. Will need to think this through and fix in a further PR, but it works decently in the interim.

The change to the protos is more or less consistent with what is described in the issue #363.

@zhilingc
Copy link
Collaborator Author

/hold cancel

@zhilingc
Copy link
Collaborator Author

/retest

sdk/python/feast/client.py Outdated Show resolved Hide resolved
sdk/python/feast/feature_set.py Outdated Show resolved Hide resolved
@zhilingc
Copy link
Collaborator Author

/retry

@woop
Copy link
Member

woop commented Dec 17, 2019

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants