Skip to content
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

WIP: OSRD goes async #7103

Closed
wants to merge 48 commits into from
Closed

WIP: OSRD goes async #7103

wants to merge 48 commits into from

Conversation

ElysaSrc
Copy link
Member

@ElysaSrc ElysaSrc commented Apr 4, 2024

This PR holds the whole modifications for making OSRD async using a RabbitMQ. This is a work in progress, here is an overview:

  • Modify local environement to provide rabbitmq
  • Implement Osrdyne
    • Implement Kubernetes driver
    • Implement Docker driver
    • Implement RabbitMQ driver
    • Implement logic
  • Test the whole stack
    • Against Docker
    • Against Kubernetes
  • Modify Core to use queues
    • Create a client library for enqueing/dequeing messages
    • Use the said library
  • Modify Editoast to use queues
    • Create a client library for enqueing/dequeing messages
    • Use the said library

Closes #6679, #6680, #6681

Helm cart related PR : OpenRailAssociation/osrd-chart#11

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.97%. Comparing base (fc0cd2e) to head (585fb3a).
Report is 41 commits behind head on dev.

❗ Current head 585fb3a differs from pull request most recent head 406f69e. Consider uploading reports for the commit 406f69e to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7103      +/-   ##
============================================
- Coverage     29.00%   27.97%   -1.03%     
  Complexity     2250     2250              
============================================
  Files          1070     1069       -1     
  Lines        132704   135480    +2776     
  Branches       2728     2746      +18     
============================================
- Hits          38487    37901     -586     
- Misses        92637    95981    +3344     
- Partials       1580     1598      +18     
Flag Coverage Δ
core 80.07% <ø> (-0.03%) ⬇️
editoast 74.80% <ø> (+0.13%) ⬆️
front 8.95% <ø> (-0.35%) ⬇️
gateway 2.65% <ø> (+0.19%) ⬆️
railjson_generator 87.15% <ø> (ø)
tests ?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ElysaSrc ElysaSrc force-pushed the ev/async-rework branch 5 times, most recently from 0aa5226 to 28d1ffa Compare April 4, 2024 18:23
@ElysaSrc ElysaSrc force-pushed the ev/async-rework branch 5 times, most recently from 5754519 to 585fb3a Compare April 5, 2024 10:11
.github/CODEOWNERS Outdated Show resolved Hide resolved
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

I haven't reviewed everything yet, but here's the first batch of comments. The PR is really good 👍

core_controller/Cargo.toml Outdated Show resolved Hide resolved
core_controller/src/config.rs Outdated Show resolved Hide resolved
core_controller/src/control_loop.rs Outdated Show resolved Hide resolved
@flomonster flomonster self-requested a review April 5, 2024 15:04
@ElysaSrc ElysaSrc force-pushed the ev/async-rework branch 2 times, most recently from bec8381 to 3972b96 Compare April 10, 2024 06:53
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! It looks good to me.

Important

Can you run cargo fmt with the following format config: #7049

Note

It's difficult for me to review the kubernetes.rs file since I'm not familiar with how k8s works.

core_controller/src/control_loop.rs Outdated Show resolved Hide resolved
core_controller/src/config.rs Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.90%. Comparing base (814e018) to head (70c0eeb).
Report is 2 commits behind head on dev.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (814e018) and HEAD (70c0eeb). Click for more details.

HEAD has 9 uploads less than BASE
Flag BASE (814e018) HEAD (70c0eeb)
railjson_generator 2 1
gateway 2 1
tests 2 0
front 2 1
core 2 0
editoast 2 0
Additional details and impacted files
@@              Coverage Diff              @@
##                dev    #7103       +/-   ##
=============================================
- Coverage     28.07%   10.90%   -17.18%     
=============================================
  Files          1288      673      -615     
  Lines        157729   113438    -44291     
  Branches       3121     1118     -2003     
=============================================
- Hits          44283    12368    -31915     
+ Misses       111569    99958    -11611     
+ Partials       1877     1112      -765     
Flag Coverage Δ
core ?
editoast ?
front 9.94% <ø> (ø)
gateway 2.52% <ø> (+0.18%) ⬆️
railjson_generator 87.49% <ø> (ø)
tests ?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ElysaSrc
Copy link
Member Author

Tested Kubernetes driver against a real kubernetes cluster deployed for the tests. Worked properly.

@ElysaSrc
Copy link
Member Author

Since I started to work on this PR, we've revamped at multiple occasion the architecture of the scalable Async RPC. We ended up on something really different that what was first envisioned.

Closing this PR in favor of a one against with a new branch.

@ElysaSrc ElysaSrc closed this Jul 14, 2024
@ElysaSrc ElysaSrc mentioned this pull request Jul 14, 2024
10 tasks
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.

Add a custom core-controller to manage specialized core instances
5 participants