Skip to content

Jobservice control loop (based on #1140) - #1156

Merged
woop merged 14 commits into
feast-dev:masterfrom
oavdeev:jobservice-control-loop
Nov 14, 2020
Merged

Jobservice control loop (based on #1140)#1156
woop merged 14 commits into
feast-dev:masterfrom
oavdeev:jobservice-control-loop

Conversation

@oavdeev

@oavdeev oavdeev commented Nov 11, 2020

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
See #1140 for start of the discussion. Compared to that PR, this addresses some comments and adds tests for the control loop logic.

Does this PR introduce a user-facing change?:

NONE

@oavdeev-tt

Copy link
Copy Markdown
Contributor

/test test-end-to-end

@oavdeev

oavdeev commented Nov 11, 2020

Copy link
Copy Markdown
Collaborator Author

/kind feature

@feast-ci-bot feast-ci-bot added kind/feature New feature or request and removed needs-kind labels Nov 11, 2020
@oavdeev
oavdeev force-pushed the jobservice-control-loop branch from a4a4dc4 to 4635773 Compare November 11, 2020 11:51
Comment thread sdk/python/tests/test_streaming_control_loop.py Outdated
Comment thread sdk/python/tests/test_streaming_control_loop.py Outdated
@oavdeev
oavdeev force-pushed the jobservice-control-loop branch 3 times, most recently from eee144c to c4c1ae2 Compare November 11, 2020 17:37
@oavdeev

oavdeev commented Nov 11, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end-gcp

@oavdeev
oavdeev force-pushed the jobservice-control-loop branch 2 times, most recently from c4c1ae2 to 5dd2490 Compare November 12, 2020 10:24
@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

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

@pyalex

pyalex commented Nov 13, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

@pyalex

pyalex commented Nov 13, 2020

Copy link
Copy Markdown
Collaborator

/test test-end-to-end-aws
/test test-end-to-end-gcp

@pyalex

pyalex commented Nov 13, 2020

Copy link
Copy Markdown
Collaborator

/test test-end-to-end-gcp

1 similar comment
@oavdeev

oavdeev commented Nov 13, 2020

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end-gcp

@feast-ci-bot feast-ci-bot removed the lgtm label Nov 13, 2020
@oavdeev
oavdeev force-pushed the jobservice-control-loop branch 3 times, most recently from dc46656 to fd877cf Compare November 13, 2020 20:08
@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/retest

2 similar comments
@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/retest

@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/retest

tsotnet and others added 12 commits November 14, 2020 19:56
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
…small fixes

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
- Move ensure_stream_ingestion_jobs to client
- Move _job_to_proto to SparkJob & its subclasses (as .to_proto methods)
- Remove list_jobs_by_hash from job launcher layer
- Add .get_hash() to StreamIngestionJob objects, which replaces list_jobs_by_hash in a much nicer way
- Add logic in dataproc launcher (completes all 3 spark modes)
- Add hash to Job proto & RemoteStreamIngestionJob
- Add bunch of docstrings

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
@woop
woop force-pushed the jobservice-control-loop branch from fd877cf to 54190e6 Compare November 14, 2020 11:57
@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/lgtm

@feast-ci-bot

Copy link
Copy Markdown
Collaborator

New changes are detected. LGTM label has been removed.

Signed-off-by: Willem Pienaar <git@willem.co>
@woop
woop force-pushed the jobservice-control-loop branch from b1ecfa7 to bf44bef Compare November 14, 2020 12:51
Signed-off-by: Willem Pienaar <git@willem.co>
@woop
woop merged commit de820be into feast-dev:master Nov 14, 2020
pyalex pushed a commit that referenced this pull request Nov 24, 2020
* Implement Job Service control loop for stream ingestion jobs

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* Update sdk/python/feast/job_service.py

Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* Update sdk/python/feast/job_service.py

Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* Ensure jobservice isn't running with failed control loop; also other small fixes

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* Code restructure (see commit message details)

- Move ensure_stream_ingestion_jobs to client
- Move _job_to_proto to SparkJob & its subclasses (as .to_proto methods)
- Remove list_jobs_by_hash from job launcher layer
- Add .get_hash() to StreamIngestionJob objects, which replaces list_jobs_by_hash in a much nicer way
- Add logic in dataproc launcher (completes all 3 spark modes)
- Add hash to Job proto & RemoteStreamIngestionJob
- Add bunch of docstrings

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* Fix super -> super()

Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>

* make _job_to_proto a function

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* move ensure_stream_ingestion_jobs back out of the client

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* add tests for the job control loop

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* fix _job_to_proto

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* fix job cache state leak between tests

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* fix docker-compose test

Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>

* Add retries to prevent download failure

Signed-off-by: Willem Pienaar <git@willem.co>

* Remove MAVEN_OPTS again

Signed-off-by: Willem Pienaar <git@willem.co>

Co-authored-by: Tsotne Tabidze <tsotnet@gmail.com>
Co-authored-by: Willem Pienaar <git@willem.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants