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

Move communication with IngestionJob to JobCoordinator #800

Merged
merged 1 commit into from
Jun 18, 2020

Conversation

pyalex
Copy link
Collaborator

@pyalex pyalex commented Jun 17, 2020

What this PR does / why we need it:

To resolve some concerns related to overloading responsibilities in SpecService in my previous PR #792 I moved all communications with IngestionJob to JobCoordinatorService

Implemented flow

New FeatureSet:

  1. FeatureSet being created through SpecService.applyFeatureSet with status PENDING
  2. JobCoordinator.Poll allocates this Set to Job(s) by creating FeatureSetJobStatus (see JobUpdateTask.updateFeatureSets) with version=0 and deliveryStatus=in-progress
  3. JobCoordinator.notifyJobs detects that there's pending FeatureSet with allocated job(s) and send FeatureSetSpec to kafka (topic is shared between all jobs)
  4. JobCoordinator.listenAckFromJobs receives ack from job and update deliveryStatus in FeatureSetJobStatus
  5. As soon as all allocated jobs acknowledged FeatureSet status set to READY

Existing FeatureSet:

  1. FeatureSet being updated through SpecService.applyFeatureSet. Status changed to PENDING. Version incremented.
  2. FeatureSet is already allocated to job(s) so JobCoordinator.notifyJobs picks it up and send to kafka with updating FeatureSetJobStatus to the latest version
    ... same flow as previous

New job spawned (bc previous canceled, eg):

  1. JobUpdateTask creates FeatureSetJobStatuses for all allocated FeatureSets with version = <latest-version-of-feature-set> since new Job can read whole history of FeatureSets from kafka topic. Nothing is being sent to specs topic

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:


@pyalex pyalex requested a review from zhilingc as a code owner June 17, 2020 08:12
@pyalex pyalex changed the title [TechDebt] Move communication with job to JobCoordinator [TechDebt] Move communication with IngestionJob to JobCoordinator Jun 17, 2020
@pyalex pyalex force-pushed the specs-in-kafka branch 2 times, most recently from 97cd674 to fb717ce Compare June 17, 2020 11:01
@pyalex pyalex requested a review from ches June 17, 2020 12:03
@pyalex
Copy link
Collaborator Author

pyalex commented Jun 18, 2020

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

@woop
Copy link
Member

woop commented Jun 18, 2020

/lgtm

@woop woop changed the title [TechDebt] Move communication with IngestionJob to JobCoordinator Move communication with IngestionJob to JobCoordinator Jun 18, 2020
@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

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

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 d3acb83 into feast-dev:master Jun 18, 2020
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