8000 feat(daemon): allow gpus arg for remote Executors by deepankarm · Pull Request #3510 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(daemon): allow gpus arg for remote Executors #3510

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 28, 2021
Merged

feat(daemon): allow gpus arg for remote Executors #3510

merged 1 commit into from
Sep 28, 2021

Conversation

deepankarm
Copy link
Contributor
@deepankarm deepankarm commented Sep 28, 2021 •
edited
Loading

This PR

  1. Enables any remote Executors to set gpus argument with Flow syntax. This argument was only valid for local + dockerized Executors. Since all remote Executors are dockerized, this enables setting up docker devices on any remote containers.
  2. All remote Flows are by default started with gpus: all devices set, as we don't have gpus arg for Flows. If needed and used, we can enable it later.

Note: If on remote, docker cannot access the device passed, we reset the device & start the container again.

Usage with remote Executor

f = Flow().add(
    uses='CustomExecutor',
    host=CLOUDHOST,
    port_jinad=8000,
    py_modules=['/path/to/pymodule.py'],
    upload_files=['/path/to/requirements.txt'],
    gpus='all'
)

@deepankarm deepankarm requested a review from a team as a code owner September 28, 2021 15:02
@github-actions github-actions bot added size/M area/core This issue/PR affects the core codebase area/daemon area/network This issue/PR affects network functionality area/testing This issue/PR affects testing component/peapod labels Sep 28, 2021
@deepankarm deepankarm requested a review from bwanglzu September 28, 2021 15:03
@github-actions
Copy link
github-actions bot commented Sep 28, 2021 •
edited
Loading

Latency summary

Current PR yields:

  • 🐢🐢 index QPS at 1195, delta to last 2 avg.: -11%
  • 🐢🐢 query QPS at 45, delta to last 2 avg.: -19%
  • 🐢🐢 dam extend QPS at 42393, delta to last 2 avg.: -18%
  • 🐢🐢 avg flow time within 1.2072 seconds, delta to last 2 avg.: -13%
  • 🐎🐎 import jina within 0.3894 seconds, delta to last 2 avg.: +7%

Breakdown

Version Index QPS Query QPS DAM Extend QPS Avg Flow Time (s) Import Time (s)
current 1195 45 42393 1.2072 0.3894
2.1.4 1111 43 37062 1.7315 0.4261
2.1.3 1581 67 66662 1.0551 0.3016

Backed by latency-tracking. Further commits will update this comment.

JoanFM
JoanFM previously approved these changes Sep 28, 2021
View reviewed changes
Copy link
Contributor
@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

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

LGTM!

@deepankarm deepankarm added the focus/ease-of-use This issue/PR affects the usability of the core label Sep 28, 2021
@codecov
Copy link
codecov bot commented Sep 28, 2021 •
edited
Loading

Codecov Report

Merging #3510 (a0021fd) into master (abd8e6c) will increase coverage by 0.35%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3510      +/-   ##
==========================================
+ Coverage   89.19%   89.54%   +0.35%     
==========================================
  Files         151      152       +1     
  Lines       11243    11261      +18     
==========================================
+ Hits        10028    10084      +56     
+ Misses       1215     1177      -38
Flag Coverage Δ
daemon 45.65% <52.38%> (+0.14%) ⬆️
jina 89.48% <95.23%> (+0.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/peapods/runtimes/container/__init__.py 80.12% <50.00%> (-1.75%) ⬇️
jina/logging/profile.py 98.19% <100.00%> (+0.01%) ⬆️
jina/peapods/runtimes/container/helper.py 88.09% <100.00%> (+7.32%) ⬆️
jina/peapods/runtimes/jinad/__init__.py 88.88% <100.00%> (ø)
jina/hubble/hubio.py 87.50% <0.00%> (ø)
jina/parsers/hubble/new.py 100.00% <0.00%> (ø)
jina/peapods/zmq/__init__.py 89.51% <0.00%> (+0.23%) ⬆️
jina/parsers/hubble/__init__.py 96.87% <0.00%> (+0.72%) ⬆️
jina/peapods/runtimes/gateway/http/app.py 92.40% <0.00%> (+1.26%) ⬆️
jina/flow/base.py 89.77% <0.00%> (+1.53%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c21d32c...a0021fd. Read the comment docs.

@deepankarm deepankarm requested a review from JoanFM September 28, 2021 16:46
@JoanFM JoanFM merged commit dd45355 into master Sep 28, 2021
@JoanFM JoanFM deleted the feat-gpuarg-remote branch September 28, 2021 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoanFM JoanFM JoanFM approved these changes

@maximilianwerk maximilianwerk Awaiting requested review from maximilianwerk

@jacobowitz jacobowitz Awaiting requested review from jacobowitz

@bwanglzu bwanglzu Awaiting requested review from bwanglzu

Assignees
No one assigned
Labels
area/core This issue/PR affects the core codebase area/daemon area/network This issue/PR affects network functionality area/testing This issue/PR affects testing component/peapod focus/ease-of-use This issue/PR affects the usability of the core size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@deepankarm @JoanFM
0