8000 fix(routing): fix static routing table args by jacobowitz · Pull Request #3363 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(routing): fix static routing table args #3363

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.

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

Merged
merged 7 commits into from
Sep 9, 2021

Conversation

jacobowitz
Copy link
Contributor

While trying to use static routing tables in kubernetes (meaning not sending it in every message), I noticed that this actually does not work when using the real CLI.
I was using the wrong pattern before (--flag ), this PR fixes this and unifies the arg to --static-routing-table.

By default this args is false and only if provided it disables sending out the routing table.

Also I fix I minor thing in the K8s pod where I need to regenerate the args on demand (because some args are calculated later).

@jacobowitz jacobowitz requested a review from a team as a code owner September 7, 2021 14:57
@jacobowitz jacobowitz requested a review from maateen September 7, 2021 14:57
@github-actions github-actions bot added size/S area/core This issue/PR affects the core codebase area/network This issue/PR affects network functionality component/flow component/peapod component/type labels Sep 7, 2021
@github-actions
Copy link
github-actions bot commented Sep 7, 2021

Latency summary

Current PR yields:

  • 🐎🐎🐎🐎 index QPS at 1188, delta to last 2 avg.: +13%
  • 🐎🐎🐎🐎 query QPS at 49, delta to last 2 avg.: +18%
  • 🐎🐎🐎🐎 dam extend QPS at 44805, delta to last 2 avg.: +9%
  • 🐎🐎🐎🐎 avg flow time within 1.7303 seconds, delta to last 2 avg.: +13%
  • 😶 import jina within 0.4496 seconds, delta to last 2 avg.: +3%

Breakdown

Version Index QPS Query QPS DAM Extend QPS Avg Flow Time (s) Import Time (s)
current 1188 49 44805 1.7303 0.4496
2.0.23 1063 41 40367 1.7534 0.4183
2.0.22 1024 41 41396 1.2983 0.4507

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

@jacobowitz jacobowitz force-pushed the fix-static-routing-table-args branch from a4d4d94 to 1693857 Compare September 7, 2021 15:14
@jacobowitz jacobowitz closed this Sep 7, 2021
@jacobowitz jacobowitz reopened this Sep 7, 2021
@github-actions github-actions bot added the area/cli This issue/PR affects the command line interface label Sep 7, 2021
@codecov
Copy link
codecov bot commented Sep 7, 2021

Codecov Report

Merging #3363 (68534ea) into master (501a9e8) will increase coverage by 6.08%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3363      +/-   ##
==========================================
+ Coverage   83.53%   89.61%   +6.08%     
==========================================
  Files         152      152              
  Lines       10840    10842       +2     
==========================================
+ Hits         9055     9716     +661     
+ Misses       1785     1126     -659     
Flag Coverage Δ
daemon 45.48% <57.14%> (-0.02%) ⬇️
jina 89.17% <85.71%> (+5.64%) ⬆️

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

Impacted Files Coverage Δ
jina/flow/base.py 80.57% <ø> (-8.47%) ⬇️
jina/parsers/__init__.py 97.87% <ø> (-0.03%) ⬇️
jina/parsers/flow.py 100.00% <ø> (ø)
jina/peapods/pods/k8s.py 90.90% <33.33%> (-1.47%) ⬇️
jina/parsers/peapods/base.py 100.00% <100.00%> (ø)
jina/peapods/grpc/__init__.py 91.20% <100.00%> (ø)
jina/peapods/runtimes/grpc/__init__.py 87.67% <100.00%> (ø)
jina/peapods/runtimes/zmq/zed.py 92.85% <100.00%> (-1.03%) ⬇️
jina/peapods/zmq/__init__.py 88.34% <100.00%> (-0.70%) ⬇️
jina/types/message/__init__.py 85.92% <100.00%> (+0.97%) ⬆️
... and 40 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 501a9e8...68534ea. Read the comment docs.

@jacobowitz jacobowitz closed this Sep 8, 2021
@jacobowitz jacobowitz reopened this Sep 8, 2021
@jacobowitz jacobowitz closed this Sep 8, 2021
@jacobowitz jacobowitz reopened this Sep 8, 2021
@jacobowitz jacobowitz force-pushed the fix-static-routing-table-args branch from 29526a3 to dd6cfb8 Compare September 8, 2021 14:49
@jacobowitz jacobowitz merged commit d78ff67 into master Sep 9, 2021
@jacobowitz jacobowitz deleted the fix-static-routing-table-args branch September 9, 2021 08:18
deepankarm pushed a commit that referenced this pull request Sep 14, 2021
* fix(routing): fix static routing table args

* fix(routing): move routing table args outside of gateway

* style: fix overload and cli autocomplete

* style: fix overload and cli autocomplete

* fix(routing): fix zmqlet static routing check

* fix(routing): fix args setting

Co-authored-by: Jina Dev Bot <dev-bot@jina.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli This issue/PR affects the command line interface area/core This issue/PR affects the core codebase area/network This issue/PR affects network functionality component/flow component/peapod component/type size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0