Fix netx.Conn metrics and reduce ndt7 websocket buffer #299
Conversation
Pull Request Test Coverage Report for Build 1370
💛 - Coveralls |
| @@ -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) | |||
| // 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. | ||
| const DefaultWebsocketBufferSize = 1 << 20 |
There was a problem hiding this comment.
Would prefer a comment after this // 1 MB or maybe // 1 MiB if you are feeling pedantic.
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @pboothe and @stephen-soltesz)
ndt7/spec/spec.go, line 30 at r1 (raw file):
Previously, pboothe (Peter Boothe) wrote…
Would prefer a comment after this
// 1 MBor maybe// 1 MiBif you are feeling pedantic.
Done. I call out 1MB explicitly in the comment now. The other constants in this file do not have annotations. So, I hope this captures the spirit of your feedback.
|
It very much captures the spirit of my feedback. The other constants also need to have units, and this seemed as good a place to start as any. Now all the other constants (that aren't unitless or durations) look a little underdocumented, which is as it should be. The next person who touches their comments will now (hopefully) put a little extra in there. |
During load testing, I discovered that ndt5 can close netx.Conns multiple times. This change guarantees that the metric is only decremented once.
During load testing, I discovered that ndt7 RAM requirements limit maximum concurrent connections to 1/3 ndt5's level with ndt-server RSS maxing out around 9.5GB before the system failed. This appears to be due to the extremely large websocket buffers. This change adds a new constant for the websocket buffer sizes of 1MB. Buffer sizes are independent of websocket message sizes.
With these settings load testing was able to comfortably support 300 concurrent connections (the previous water mark from NDT5) or ~1700 tests/min (very close to the theoretical limit of 1800 / min for 10sec tests, which is much tighter run time than ndt5's 15sec/test).
This change is