Skip to content

Make it more explict that resources are per worker.#2470

Merged
mrocklin merged 4 commits intodask:masterfrom
lesteve:doc-resources-per-worker
Jan 28, 2019
Merged

Make it more explict that resources are per worker.#2470
mrocklin merged 4 commits intodask:masterfrom
lesteve:doc-resources-per-worker

Conversation

@lesteve
Copy link
Member

@lesteve lesteve commented Jan 20, 2019

Although it is mentioned in the text already, I was confused between the dask-worker command (that can create multiple workers) vs "worker".

I think that adding a simple example with dask-worker --nprocs helps clearing the suggestion. Suggestions on how to improve it more than welcome!

For more context, some (long) discussion happened in dask/dask-jobqueue#181 about how to execute tasks that managed threading internally (in the OP python bindings on top of C++ code that does multi-threading) and some concerns about running non thread-safe tasks in combination with resources. I have to say it took me a while to figure out that resources could be used to solve the problem.

Add example for how to execute non thread-safe tasks.
@mrocklin
Copy link
Member

Other than the small comment this looks good to me. Thanks @lesteve

Also I raised #2471 about removing --nprocs generally, which seems to have a lot of ambiguously defined behavior. Your thoughts would be particularly welcome there.

Copy link
Member

@guillaumeeb guillaumeeb left a comment

Choose a reason for hiding this comment

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

@lesteve
Copy link
Member Author

lesteve commented Jan 23, 2019

I tackled all the comments I think. Let me know if something can be improved!

@lesteve
Copy link
Member Author

lesteve commented Jan 23, 2019

I am only changing documentation so I bet the Travis error is not related to this PR.

Copy link
Member

@guillaumeeb guillaumeeb left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @lesteve.

@mrocklin mrocklin merged commit 1c6b2e8 into dask:master Jan 28, 2019
@lesteve lesteve deleted the doc-resources-per-worker branch January 29, 2019 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants