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

snet: Path lookup in snet.Write greatly degrades performance when called in parallel #1702

Closed
scrye opened this issue Jul 20, 2018 · 0 comments · Fixed by #2145
Closed

snet: Path lookup in snet.Write greatly degrades performance when called in parallel #1702

scrye opened this issue Jul 20, 2018 · 0 comments · Fixed by #2145
Assignees
Milestone

Comments

@scrye
Copy link
Contributor

scrye commented Jul 20, 2018

Whenever an application writes via snet.Conn to a remote address with an unset path, snet will try to retrieve the path from SCIOND. This blocks the calling goroutine, but also any other goroutine trying to write, even if they might want to send data via a known path.

This needs to be fixed s.t. snet only blocks the calling goroutine. One possible solution is to move path resolution outside of the global internal lock.

Note that this can be tricky to handle when callers change the write deadline of the socket (via SetWriteDeadline or SetDeadline) to some time in the past, as in this case net.PacketConn dictates that the call should return (see https://golang.org/pkg/net/#PacketConn):

// A deadline is an absolute time after which I/O operations
// fail with a timeout (see type Error) instead of
// blocking. The deadline applies to all future and pending
// I/O, not just the immediately following call to ReadFrom or
// WriteTo. After a deadline has been exceeded, the connection
// can be refreshed by setting a deadline in the future.

In other words, the path request should be canceled when the deadline is exceeded, including the case where the deadline has been changed after the call to get paths. This might mean maintaining a queue of cancelFuncs that tracks active SCIOND queries.

This requires #1701 to be implemented first.

@scrye scrye changed the title snet: Path lookup in snet.Write greately degrades performance when called in parallel snet: Path lookup in snet.Write greatly degrades performance when called in parallel Jul 20, 2018
@scrye scrye added the snet label Jul 20, 2018
@shitz shitz added this to the Sprint #6 milestone Jul 23, 2018
@shitz shitz modified the milestones: Sprint #6, Sprint #7 Aug 13, 2018
@lukedirtwalker lukedirtwalker modified the milestones: Sprint #7, Sprint #8 Sep 3, 2018
@shitz shitz removed this from the Sprint #8 milestone Sep 24, 2018
@shitz shitz added this to the Sprint #11 milestone Nov 12, 2018
scrye added a commit that referenced this issue Nov 16, 2018
This allows safer refactoring in the future (and paves the way for #1702).
@scrye scrye reopened this Nov 21, 2018
scrye added a commit that referenced this issue Nov 23, 2018
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 a pull request may close this issue.

3 participants