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

Creates new stream connection when an address is in the "neighbours" list #28

Merged
merged 19 commits into from
Apr 1, 2020

Conversation

nkcr
Copy link
Contributor

@nkcr nkcr commented Mar 26, 2020

Now instead of always asking the orchestrator to relay messages, each server has a routing table and a "mesh" list, which is a list of server it must create a connection to. The routing table tells a server that doesn't have a direct connection to a peer which peer it can contact to relay that message.

I also implemented an optional way of saving all the trafic in a server. This is convenient in the tests to check that everything went well. It also provides interesting insights on how things work.

@nkcr nkcr added enhancement New feature or request mod/mino About the Mino module labels Mar 26, 2020
@nkcr nkcr self-assigned this Mar 26, 2020
@nkcr nkcr added the R4M 🚀 The PR is ready to be reviewed and merged label Mar 27, 2020
@nkcr nkcr requested a review from Gilthoniel March 27, 2020 11:55
go.mod Outdated Show resolved Hide resolved
mino/minogrpc/mod.go Outdated Show resolved Hide resolved
mino/minogrpc/overlay.go Outdated Show resolved Hide resolved
mino/minogrpc/server.go Outdated Show resolved Hide resolved
mino/minogrpc/server.go Outdated Show resolved Hide resolved
mino/minogrpc/overlay.go Outdated Show resolved Hide resolved
mino/minogrpc/overlay.go Outdated Show resolved Hide resolved
mino/minogrpc/server.go Outdated Show resolved Hide resolved
@nkcr
Copy link
Contributor Author

nkcr commented Mar 31, 2020

@Gilthoniel made the changes as discussed. You may want to first have a look at this test in order to check the logic:
https://github.com/dedis/fabric/blob/dfc4afe8f03e7d1721fb6e384dbe8bb107bbb7b6/mino/minogrpc/server_test.go#L962

@nkcr nkcr requested a review from Gilthoniel March 31, 2020 07:32
mino/minogrpc/overlay.go Show resolved Hide resolved
mino/minogrpc/server.go Show resolved Hide resolved
mino/minogrpc/server.go Show resolved Hide resolved
mino/minogrpc/server.go Outdated Show resolved Hide resolved
mino/minogrpc/server_test.go Outdated Show resolved Hide resolved
context string
}

func (p item) String() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we define the String function to be a debug/log function that is minimalistic and one-liner (because %v is using it) and define a Display(io.Writer) function (in encoding for instance) that displays all the details like below ? So you can simply do item.Display(os.Stdout) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good idea

@Gilthoniel
Copy link
Contributor

Gilthoniel commented Mar 31, 2020

@nkcr I don't know if the mesh/routing tables are temporary and you planned to improve this later so if that's the case, you can ignore the comments related to it.

@nkcr
Copy link
Contributor Author

nkcr commented Mar 31, 2020

Yes the routing/mesh structs are only temporary solutions. I agree this should be done in a per request fashion.

A later job would be to have a deterministic flag to allow that kind of checking
We keep .String() for short printing
@nkcr nkcr requested a review from Gilthoniel March 31, 2020 09:44
@Gilthoniel Gilthoniel merged commit b550e7b into master Apr 1, 2020
@Gilthoniel Gilthoniel deleted the minogrpc-mesh branch April 1, 2020 07:53
bbjubjub2494 pushed a commit to bbjubjub2494/dela that referenced this pull request Aug 3, 2024
Creates new stream connection when an address is in the "neighbours" list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request mod/mino About the Mino module R4M 🚀 The PR is ready to be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants