Skip to content

FeatureSets are delivered to Ingestion Job through Kafka#792

Merged
feast-ci-bot merged 2 commits intofeast-dev:masterfrom
pyalex:specs-in-kafka
Jun 17, 2020
Merged

FeatureSets are delivered to Ingestion Job through Kafka#792
feast-ci-bot merged 2 commits intofeast-dev:masterfrom
pyalex:specs-in-kafka

Conversation

@pyalex
Copy link
Copy Markdown
Collaborator

@pyalex pyalex commented Jun 11, 2020 •
edited
Loading

What this PR does / why we need it:

SpecService & IngestionJob are now communicate through kafka topics, which makes job restarts on FeatureSet change obsolete. Now job restarted only when subscription configuration of store was changed.

Communication Flow:

  1. SpecService.applyFeatureSet publish FeatureSetSpec to specs-topic and set FeatureSet status to Pending
  2. IngestionJob reads from this topic (all history of changes + recent updates) & build materialized view of Specs in memory
  3. IngestionJob sends acknowledgment back to SpecService via specs-ack-topic
  4. SpecService collects acknowledgments from all related jobs (see FeatureSetJobStatus) and when all running jobs acknowledged FeatureSet status is changed to Ready

Which issue(s) this PR fixes:

Fixes #761

Does this PR introduce a user-facing change?:
No

@pyalex pyalex changed the title [WIP] FeatureSets are delivered to Ingestion Job through Kafka FeatureSets are delivered to Ingestion Job through Kafka Jun 12, 2020
@pyalex pyalex added the kind/feature New feature or request label Jun 12, 2020
@pyalex
Copy link
Copy Markdown
Collaborator Author

pyalex commented Jun 12, 2020

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

woop
woop reviewed Jun 12, 2020
View reviewed changes
woop
woop reviewed Jun 15, 2020
View reviewed changes
woop
woop reviewed Jun 15, 2020
View reviewed changes
woop
woop reviewed Jun 15, 2020
View reviewed changes
core/src/main/java/feast/core/service/SpecService.java Outdated
Copy link
Copy Markdown
Member

@woop woop Jun 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to flag that even though I agree with the approach we are taking with Spring and Kafka, I do consider it technical debt that we are going into. Ideally the life cycle of jobs and the updates of feature sets to those jobs would fully encapsulated in the job management layer, especially if we ever want to separate job management from core.

Copy link
Copy Markdown
Collaborator Author

@pyalex pyalex Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be more than happy to move this communication responsibility to JobService, but right now JobService is dependant on SpecService and I need publishing to kafka be synchronous part of applyFeatureSet. So yeah, currently it's a tech debt.

woop
woop reviewed Jun 15, 2020
View reviewed changes
@pyalex
Copy link
Copy Markdown
Collaborator Author

pyalex commented Jun 16, 2020

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

woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
woop
woop reviewed Jun 16, 2020
View reviewed changes
@pyalex
intermediate table in m2m Job <-> FeatureSet with delivery status + F…
1e3fde0 
…eatureSet version

switch to spring-kafka (configs)

specService send message to kafka & expect ack & update status accordingly

jobs runner to send source & specs config (source + ack)

ingestion job to read specs from kafka and send ack

return featureSets in ingestionJob

generate uniq topic name for each test run

prevent listJobs from failing when job failed on start
@pyalex
Copy link
Copy Markdown
Collaborator Author

pyalex commented Jun 16, 2020

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

@woop
Copy link
Copy Markdown
Member

woop commented Jun 17, 2020

/lgtm

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Jun 17, 2020

[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

Details 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 6137102 into feast-dev:master Jun 17, 2020
@pyalex pyalex mentioned this pull request Jun 17, 2020
Merged
@woop woop mentioned this pull request Jun 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop approved these changes

@davidheryanto davidheryanto Awaiting requested review from davidheryanto

@khorshuheng khorshuheng Awaiting requested review from khorshuheng

@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@woop woop

Labels

approved kind/feature New feature or request lgtm size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Job Coordination Improvement Proposal

3 participants

@pyalex @woop @feast-ci-bot