Skip to content

Add dataproc executor resource config - #1160

Merged
feast-ci-bot merged 5 commits into
feast-dev:masterfrom
terryyylim:dataproc-resource-config
Nov 19, 2020
Merged

Add dataproc executor resource config#1160
feast-ci-bot merged 5 commits into
feast-dev:masterfrom
terryyylim:dataproc-resource-config

Conversation

@terryyylim

@terryyylim terryyylim commented Nov 13, 2020

Copy link
Copy Markdown
Member

Signed-off-by: Terence terencelimxp@gmail.com

What this PR does / why we need it:
Currently one job can allocate too many resources (by default it will take amount of executors equal to partitions), which can break parallelization. This PR allows configuring of resource allocation per one job, so that many jobs can share a cluster.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Users can now configure amount of resources to allocate for their spark ingestion jobs.

@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/retest

1 similar comment
@woop

woop commented Nov 14, 2020

Copy link
Copy Markdown
Member

/retest

woop
woop reviewed Nov 14, 2020
View reviewed changes
Comment thread sdk/python/feast/pyspark/launchers/gcloud/dataproc.py Outdated

@woop woop Nov 14, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why positional arguments? Are these required? We don't have defaults?

@woop

woop commented Nov 15, 2020

Copy link
Copy Markdown
Member

Users can now configure amount of resources to allocate for their spark ingestion jobs.

How do I as a user know how to set this configuration and what the possible options are?

@terryyylim
terryyylim force-pushed the dataproc-resource-config branch 2 times, most recently from 66095dc to bbf7ba1 Compare November 17, 2020 02:56
@pyalex

pyalex commented Nov 17, 2020

Copy link
Copy Markdown
Collaborator

@woop I think it's not for users, but rather for feast admins and those options will be configured on jobservice side

@terryyylim
terryyylim force-pushed the dataproc-resource-config branch from bbf7ba1 to 037cd6e Compare November 17, 2020 05:13
@woop

woop commented Nov 17, 2020

Copy link
Copy Markdown
Member

@woop I think it's not for users, but rather for feast admins and those options will be configured on jobservice side

Yea I know, but my question is how do I know what configuration can be set? We need to start documenting the configuration options.

@terryyylim

terryyylim commented Nov 17, 2020

Copy link
Copy Markdown
Member Author

/retest

@terryyylim
terryyylim force-pushed the dataproc-resource-config branch from 037cd6e to a2ca39b Compare November 17, 2020 07:33
pyalex
pyalex reviewed Nov 17, 2020
View reviewed changes
Comment thread tests/e2e/conftest.py Outdated
Comment thread sdk/python/feast/pyspark/launchers/gcloud/dataproc.py Outdated

@pyalex pyalex Nov 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we rather rely on defaults in constants.py than here?

Comment thread sdk/python/feast/pyspark/launcher.py Outdated

@pyalex pyalex Nov 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please use named arguments when amount of arguments is so high

@terryyylim
terryyylim force-pushed the dataproc-resource-config branch 2 times, most recently from f13bde2 to 8c809a4 Compare November 17, 2020 09:31
@terryyylim
Add dataproc executor resource config
5d48562 
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
Add default spark job executor values
7c47a4d 
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
Fix e2e tests
41c6818 
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
Shift spark configurations
ff534fd 
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
terryyylim force-pushed the dataproc-resource-config branch from 8c809a4 to ff534fd Compare November 19, 2020 04:48
@terryyylim
Update constants and docstrings
93db247 
Signed-off-by: Terence <terencelimxp@gmail.com>
@feast-ci-bot

feast-ci-bot commented Nov 19, 2020

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pyalex, terryyylim

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 19, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

@feast-ci-bot
feast-ci-bot merged commit 4d67fbb into feast-dev:master Nov 19, 2020
pyalex pushed a commit that referenced this pull request Nov 24, 2020
@terryyylim @pyalex
Add dataproc executor resource config (#1160)
89871f1 
* Add dataproc executor resource config

Signed-off-by: Terence <terencelimxp@gmail.com>

* Add default spark job executor values

Signed-off-by: Terence <terencelimxp@gmail.com>

* Fix e2e tests

Signed-off-by: Terence <terencelimxp@gmail.com>

* Shift spark configurations

Signed-off-by: Terence <terencelimxp@gmail.com>

* Update constants and docstrings

Signed-off-by: Terence <terencelimxp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop left review comments
@pyalex pyalex pyalex 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

@pyalex pyalex

Labels

approved kind/housekeeping lgtm size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@terryyylim @woop @pyalex @feast-ci-bot