8000 fix: close already started Peas when one of them fail to start by JoanFM · Pull Request #1863 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: close already started Peas when one of them fail to start #1863

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
Feb 4, 2021
Merged

fix: close already started Peas when one of them fail to start #1863

merged 1 commit into from
Feb 4, 2021

Conversation

JoanFM
Copy link
Contributor
@JoanFM JoanFM commented Feb 4, 2021 •
edited
Loading

Changes introduced
Right now when a Pea fails to start, the rest of the already started Peas will be left alive

@JoanFM JoanFM requested a review from a team as a code owner February 4, 2021 12:25
@JoanFM JoanFM linked an issue Feb 4, 2021 that may be closed by this pull request
BasePod not closing all Peas in Pod when one of the Peas fails to start (at least in remote) #1861
Closed
@jina-bot jina-bot added size/S area/core This issue/PR affects the core codebase area/network This issue/PR affects network functionality component/peapod labels Feb 4, 2021
@JoanFM JoanFM marked this pull request as draft February 4, 2021 12:28
@github-actions
Copy link
github-actions bot commented Feb 4, 2021 •
edited
Loading

Latency summary

Current PR yields:

  • 😶 index QPS at 1129, delta to last 3 avg.: -4%
  • 😶 query QPS at 24, delta to last 3 avg.: +5%

Breakdown

Version Index QPS Query QPS
current 1129 24
0.9.28 1178 22
0.9.27 1182 23

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

@JoanFM JoanFM force-pushed the fix-pod-close-started-peas branch from 92f3264 to d1c7104 Compare February 4, 2021 12:47
@JoanFM JoanFM marked this pull request as ready for review February 4, 2021 12:47
@jina-bot jina-bot added the area/testing This issue/PR affects testing label Feb 4, 2021
@JoanFM JoanFM requested review from hanxiao and nan-wang February 4, 2021 12:48
@JoanFM JoanFM force-pushed the fix-pod-close-started-peas branch from d1c7104 to a22bd1a Compare February 4, 2021 12:50
@codecov
Copy link
codecov bot commented Feb 4, 2021 •
edited
Loading

Codecov Report

Merging #1863 (837e25b) into master (11eb7c9) will increase coverage by 0.04%.
The diff coverage is 97.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1863      +/-   ##
==========================================
+ Coverage   85.95%   86.00%   +0.04%     
==========================================
  Files         145      146       +1     
  Lines        6943     6980      +37     
==========================================
+ Hits         5968     6003      +35     
- Misses        975      977       +2
Impacted Files Coverage Δ
jina/peapods/runtimes/asyncio/rest/app.py 78.91% <0.00%> (+1.36%) ⬆️
jina/proto/jina_pb2.py 100.00% <ø> (ø)
jina/types/sets/document.py 94.11% <93.33%> (+0.65%) ⬆️
jina/types/mixin.py 96.29% <96.29%> (ø)
jina/drivers/cache.py 93.75% <100.00%> (ø)
jina/drivers/control.py 95.08% <100.00%> (ø)
jina/drivers/querylang/reverse.py 100.00% <100.00%> (ø)
jina/drivers/querylang/select.py 89.65% <100.00%> (ø)
jina/drivers/querylang/sort.py 100.00% <100.00%> (ø)
jina/drivers/search.py 96.49% <100.00%> (ø)
... and 14 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 11eb7c9...e31a781. Read the comment docs.

@JoanFM JoanFM force-pushed the fix-pod-close-started-peas branch 2 times, most recently from c1ba191 to 044f00d Compare February 4, 2021 13:20
@JoanFM JoanFM force-pushed the fix-pod-close-started-peas branch from 044f00d to e31a781 Compare February 4, 2021 13:30
@hanxiao hanxiao merged commit 52731fb into master Feb 4, 2021
@hanxiao hanxiao deleted the fix-pod-close-started-peas branch February 4, 2021 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hanxiao hanxiao hanxiao approved these changes

@Roshanjossey Roshanjossey Awaiting requested review from Roshanjossey

@florian-hoenicke florian-hoenicke Awaiting requested review from florian-hoenicke

@nan-wang nan-wang Awaiting requested review from nan-wang

@cristianmtr cristianmtr Awaiting requested review from cristianmtr

@deepankarm deepankarm Awaiting requested review from deepankarm

Assignees
No one assigned
Labels
area/core This issue/PR affects the core codebase area/network This issue/PR affects network functionality area/testing This issue/PR affects testing component/peapod size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BasePod not closing all Peas in Pod when one of the Peas fails to start (at least in remote)
3 participants
@JoanFM @hanxiao @jina-bot
0