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

Make dask-gateway-server's golang proxy pass tests when built with golang 1.22+ #848

Open
consideRatio opened this issue Jan 20, 2025 · 2 comments · May be fixed by #873
Open

Make dask-gateway-server's golang proxy pass tests when built with golang 1.22+ #848

consideRatio opened this issue Jan 20, 2025 · 2 comments · May be fixed by #873
Labels
maintenance

Comments

@consideRatio
Copy link
Collaborator

dask-gateway-server ships with a golang binary, but I've failed to make it build with golang 1.22 and 1.23 in a way that makes tests pass still.

To reproduce:

# ensure you have golang 1.22 or 1.23
go version

# rebuild dask-gateway-server's proxy binary by doing this
pip install "./dask-gateway-server"

# run a test known to fail
pytest tests/test_client.py::test_cluster_widget
@consideRatio consideRatio added maintenance help wanted Extra attention is needed labels Jan 20, 2025
@consideRatio consideRatio mentioned this issue Feb 3, 2025
Merged
@consideRatio
Copy link
Collaborator Author

consideRatio commented Feb 5, 2025 •
edited
Loading

The issue in golang 1.20.0+ is that io.Copy in proxyConnections blocks and never returns:

dask-gateway/dask-gateway-server/dask-gateway-proxy/cmd/dask-gateway-proxy/main.go

Lines 528 to 535 in 9b8c0fc

func (p *Proxy) proxyConnections(wg *sync.WaitGroup, in, out sni.TcpConn) {
defer wg.Done()
if _, err := io.Copy(in, out); err != nil {
p.logger.Debugf("Error proxying %q -> %q: %s", in.RemoteAddr(), out.RemoteAddr(), err)
}
in.CloseRead()
out.CloseWrite()
}

proxyConnections is called in the end of handleConnection, which in turn gets stuck on wg.Wait():

dask-gateway/dask-gateway-server/dask-gateway-proxy/cmd/dask-gateway-proxy/main.go

Lines 521 to 525 in 9b8c0fc

var wg sync.WaitGroup
wg.Add(2)
go p.proxyConnections(&wg, pInConn, outConn.(*net.TCPConn))
go p.proxyConnections(&wg, outConn.(*net.TCPConn), pInConn)
wg.Wait()

@consideRatio consideRatio linked a pull request Feb 6, 2025 that will close this issue
Open
@consideRatio
Copy link
Collaborator Author

@consideRatio consideRatio removed the help wanted Extra attention is needed label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Migrate go code to function in go 1.22+ consideRatio/dask-gateway
1 participant
@consideRatio