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

Use SO_REUSEADDR and SO_REUSEPORT options when creating the sql server on Unix #1830

Merged
merged 4 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type Listener struct {
// For unix socket connection, 'unixSocketPath' takes a path for the unix socket file.
// If 'unixSocketPath' is empty, no need to create the second listener.
func NewListener(protocol, address string, unixSocketPath string) (*Listener, error) {
netl, err := net.Listen(protocol, address)
netl, err := newNetListener(protocol, address)
if err != nil {
return nil, err
}
Expand Down
39 changes: 39 additions & 0 deletions server/net_listener_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//go:build !windows
nicktobey marked this conversation as resolved.
Show resolved Hide resolved

package server

import (
"context"
"net"
"syscall"

"golang.org/x/sys/unix"
)

// Very rarely in our CI, the server fails to bind to the port with the error: "port already in use."
// This is odd because the server already confirms that the port is not in use before connecting.
// Using the SO_REUSEADDR and SO_REUSEPORT options prevents this spurious failure.
// This is safe to do because we have already checked that the
func newNetListener(protocol, address string) (net.Listener, error) {
lc := net.ListenConfig{
Control: func(network, address string, c syscall.RawConn) error {
var socketErr error
err := c.Control(func(fd uintptr) {
err := unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) Would be nice to leave a short one sentence comment in here quickly explaining the intent of this. Would be helpful for future code readers who find this and aren't sure why it's needed. (More helpful to explain the issue we were seeing with port reuse and the hypothesis about TIME_WAIT than just rehashing what the options do, since that's easy to look up.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

if err != nil {
socketErr = err
}

err = unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT, 1)
if err != nil {
socketErr = err
}
})
if err != nil {
return err
}
return socketErr
},
}
return lc.Listen(context.Background(), protocol, address)
}
7 changes: 7 additions & 0 deletions server/net_listener_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package server
nicktobey marked this conversation as resolved.
Show resolved Hide resolved

import "net"

func newNetListener(protocol, address string) (net.Listener, error) {
return net.Listen(protocol, address)
}
17 changes: 17 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package server

import (
"errors"
"fmt"
"net"
"time"

"github.com/dolthub/vitess/go/mysql"
Expand Down Expand Up @@ -97,6 +99,16 @@ func NewValidatingServer(
return newServerFromHandler(cfg, e, sm, handler)
}

func portInUse(hostPort string) bool {
timeout := time.Second
conn, _ := net.DialTimeout("tcp", hostPort, timeout)
if conn != nil {
defer conn.Close()
return true
}
return false
}

func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handler mysql.Handler) (*Server, error) {
if cfg.ConnReadTimeout < 0 {
cfg.ConnReadTimeout = 0
Expand All @@ -109,6 +121,11 @@ func newServerFromHandler(cfg Config, e *sqle.Engine, sm *SessionManager, handle
}

var unixSocketInUse error

if portInUse(cfg.Address) {
unixSocketInUse = fmt.Errorf("Port %s already in use.", cfg.Address)
}

l, err := NewListener(cfg.Protocol, cfg.Address, cfg.Socket)
if err != nil {
if errors.Is(err, UnixSocketInUseError) {
Expand Down