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

Fix netx.Conn metrics and reduce ndt7 websocket buffer #299

Merged
merged 4 commits into from Jun 16, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions ndt7/handler/handler.go
Expand Up @@ -126,8 +126,8 @@ func setupConn(writer http.ResponseWriter, request *http.Request) *websocket.Con
CheckOrigin: func(r *http.Request) bool {
return true // Allow cross origin resource sharing
},
ReadBufferSize: spec.MaxMessageSize,
WriteBufferSize: spec.MaxMessageSize,
ReadBufferSize: spec.DefaultWebsocketBufferSize,
WriteBufferSize: spec.DefaultWebsocketBufferSize,
}
conn, err := upgrader.Upgrade(writer, request, headers)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions ndt7/spec/spec.go
Expand Up @@ -22,6 +22,13 @@ const MaxMessageSize = 1 << 24
// a good compromise between Go and JavaScript as seen in cloud based tests.
const MaxScaledMessageSize = 1 << 20

// DefaultWebsocketBufferSize is the read and write buffer sizes used when
// creating a websocket connection. This size is independent of the websocket
// message sizes defined above (which may be larger) and used to optimize read
// and write operations. However, larger buffers will practically limit the
// total number of concurrent connections possible. We use 1MB as a balance.
const DefaultWebsocketBufferSize = 1 << 20
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer a comment after this // 1 MB or maybe // 1 MiB if you are feeling pedantic.


// ScalingFraction sets the threshold for scaling binary messages. When
// the current binary message size is <= than 1/scalingFactor of the
// amount of bytes sent so far, we scale the message. This is documented
Expand Down
4 changes: 3 additions & 1 deletion netx/net.go
Expand Up @@ -5,6 +5,7 @@ import (
"log"
"net"
"os"
"sync"
"time"

guuid "github.com/google/uuid"
Expand Down Expand Up @@ -51,6 +52,7 @@ type Conn struct {
net.Conn
fp *os.File
netinfo iface.NetInfo
once sync.Once
}

// Addr supports the net.Addr interface and allows mediated access to operations
Expand Down Expand Up @@ -110,7 +112,7 @@ func (ln *Listener) Accept() (net.Conn, error) {
// returned by LocalAddr and RemoteAddr should be released before calling Close.
func (mc *Conn) Close() error {
mc.fp.Close()
CurrentOpenConns.Dec()
mc.once.Do(CurrentOpenConns.Dec)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this.

return mc.Conn.Close()
}

Expand Down