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.

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

use mio for Tcp management in bootstrap system #3780

Merged
merged 103 commits into from
May 3, 2023
Merged

use mio for Tcp management in bootstrap system #3780

merged 103 commits into from
May 3, 2023

Conversation

modship
Copy link
Member

@modship modship commented Apr 6, 2023 •
edited by Ben-PH
Loading

  • document all added functions
  • try in sandbox /simulation/labnet
  • unit tests on the added/changed features
    • make tests compile
    • make tests pass
  • add logs allowing easy debugging in case the changes caused problems
  • if the API has changed, update the API specification

Ben-PH and others added 30 commits March 3, 2023 15:26
@Ben-PH
Make establisher get_listener non-async
7ae5b2d
@Ben-PH
Make establisher fn get_connector non-async
bdd8e54
@Ben-PH
Set the mockers to use a TcpStream as a duplex
b77875c
@Ben-PH
fix(bootstrap) : fix base_bootstrap_ip
4e6c89f
@Ben-PH
Duplex -> tokio::net::TcpStream. same for Listener
bf0301c
@Ben-PH
unify listener interface through trait
baac844
@Ben-PH
remove #[cfg(test)] from bootstrap establisher
1e00ce4
@Ben-PH
Add FIXME to where static lifetime is being used as a hot-fix
77d0b8e
@Ben-PH
Light house-keeping
710a8c8
@Ben-PH
Clear clippy lint
4d14a09
@Ben-PH
Some code-comments
7c131db
@Ben-PH
Using a blocking tcp interface, but things lock up
2150b03
@Ben-PH
Fix blocking by making the tokio conversion using non-blocking objects
2e086c1
@Ben-PH
Remove the async Tcp stuff from mock connector
a637e00
@Ben-PH
Use less async channels. Use Arc<AtomicBool> as signaller
725902f
@Ben-PH
Fewer async channels
0236535
@Ben-PH
So close to not using tokio...
b32246b
@Ben-PH
accept method now synchronous!!!
a7f3c51
@Ben-PH
Merge remote-tracking branch 'origin/testnet_21' into bootstrap/sync-…
c1c0936 
…accept
@Ben-PH
make connect synchronous
e8d53f1
@Ben-PH
Some cleanup
2cc4256
@Ben-PH
Clear clippy lint
fcdf28e
@Ben-PH
Fix upstream merge error
5732529
@Ben-PH
more cleanup
bf7f89f
@Ben-PH
Make listener blocking
78459e0
@Ben-PH
Make the Duplex a trait, thus mockable
dffa060
@Ben-PH
Start the server with the listener instead of estab trait
ea51448
@Ben-PH
Update comments
436517b
@Ben-PH
Remove mock-establisher interface entirely
11a3b24
@Ben-PH
Move the default get-listener into a struct method
e0a3089
@modship
Copy link
Member Author

modship commented Apr 6, 2023

@Ben-PH i think etablisher.rs can be deleted

@Ben-PH Ben-PH changed the base branch from bootstrap/mocked-and-sync to testnet_22 April 19, 2023 16:07
@modship modship marked this pull request as ready for review May 2, 2023 08:45
@Ben-PH
Copy link
Contributor

Ben-PH commented May 2, 2023

@modship d25f902 should fix the event-loop breakout being blocked.

Ben-PH
Ben-PH reviewed May 2, 2023
View reviewed changes
massa-bootstrap/src/server.rs Outdated Show resolved Hide resolved
massa-bootstrap/src/tests/scenarios.rs Outdated Show resolved Hide resolved
massa-bootstrap/src/tests/scenarios.rs Outdated Show resolved Hide resolved
Ben-PH
Ben-PH approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@Ben-PH Ben-PH left a comment •
edited
Loading

Choose a reason for hiding this comment

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

With follow up issues, lgtm. Nice work, @modship

EDIT: #3890

massa-bootstrap/src/listener.rs Outdated Show resolved Hide resolved
massa-bootstrap/src/listener.rs Show resolved Hide resolved
massa-bootstrap/src/listener.rs Show resolved Hide resolved
massa-bootstrap/src/listener.rs Show resolved Hide resolved
massa-bootstrap/src/server.rs Show resolved Hide resolved
massa-bootstrap/src/server.rs Show resolved Hide resolved
@Ben-PH Ben-PH mentioned this pull request May 2, 2023
Open
@modship
Copy link
Member Author

modship commented May 2, 2023

With follow up issues, lgtm. Nice work, @modship

Thanks @Ben-PH , u too ✌️

AurelienFT
AurelienFT approved these changes May 3, 2023
View reviewed changes
Copy link
Contributor

@AurelienFT AurelienFT left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems ok to me if it has been tested on labnet i'm ok to merge. If you need help to fix conflicts related to network refactoring ping me on discord.

@Ben-PH Ben-PH merged commit 66c26d6 into testnet_22 May 3, 2023
@AurelienFT AurelienFT deleted the bootstrap/listener branch May 29, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AurelienFT AurelienFT AurelienFT approved these changes

@Ben-PH Ben-PH Ben-PH approved these changes

Assignees

@modship modship

Labels
bootstrap Issues related to the bootstrap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bootstrap improvements tracking issue
3 participants
@modship @Ben-PH @AurelienFT