Make FillUntil errors non-fatal

[stephen-soltesz]

While investigating a higher than expected error rate from the throttled ndt5-client used in the e2e monitoring with access tokens, I discovered that the cause of failure from the client perspective was the server handling FillUntil errors as fatal.

FillUntil in prometheus metrics is the second most frequent error from clients. In the case of the ndt5-client, the error is a result of two steps:

• a slow client causes the ndt-server to stall in ws.WritePreparedMessage() .
• the client adds a timeout of 15sec to testconn, and the downloader goroutine runs until a read error or timeout.

After the 15sec timeout, the client closes the test connection. Because the server was blocked in a write, the write returns an error and returns from FillUntil. When the server aborts the test, the client also registers a failure.

This PR prevents that failure by ignoring the FillUntil error and continuing to collect connection metrics and sending them to the client via the control channel. If the client has truly disconnected, there are additional opportunities for error: sending results, receiving client performance, sending all metrics, sending test finalize.

---

This change is 

[coveralls]

Pull Request Test Coverage Report for Build 1279

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.3%) to 79.932%

---

Files with Coverage Reduction	New Missed Lines	%
ndt5/c2s/c2s.go	1	72.32%

Totals
Change from base Build 1275:	0.3%
Covered Lines:	1649
Relevant Lines:	2063

---

💛 - Coveralls

[stephen-soltesz]

@pboothe I think you're the right person to review this. I won't expect a response until Tuesday.

[pboothe]

LGTM once the SetWriteDeadline stuff is removed.

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @pboothe and @stephen-soltesz)

---

ndt5/protocol/protocol.go, line 184 at r3 (raw file):

func (ws *wsConnection) FillUntil(t time.Time, bytes []byte) (bytesWritten int64, err error) {
	ws.SetWriteDeadline(t)

SetWriteDeadline is a deep deep rabbithole, and I have been burned by it in the past. After talking in person, we agreed to remove those changes from this PR.

https://golang.org/src/net/net.go?s=8402:8452#L248
https://golang.org/src/net/fd_unix.go?h=netFD#L19
https://golang.org/src/internal/poll/fd_poll_runtime.go?s=3141:3190#L27
https://github.com/golang/go/blob/master/src/runtime/netpoll.go#L241

---

ndt5/s2c/s2c.go, line 99 at r3 (raw file):

	record.StartTime = time.Now()
	testConn.FillUntil(time.Now().Add(10*time.Second), dataToSend)
	record.EndTime = time.Now()

The changes to S2C look good to me.

[stephen-soltesz]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @pboothe)

---

ndt5/protocol/protocol.go, line 184 at r3 (raw file):

Previously, pboothe (Peter Boothe) wrote…

SetWriteDeadline is a deep deep rabbithole, and I have been burned by it in the past. After talking in person, we agreed to remove those changes from this PR.

https://golang.org/src/net/net.go?s=8402:8452#L248
https://golang.org/src/net/fd_unix.go?h=netFD#L19
https://golang.org/src/internal/poll/fd_poll_runtime.go?s=3141:3190#L27
https://github.com/golang/go/blob/master/src/runtime/netpoll.go#L241

Removed.

[stephen-soltesz]

https://github.com/m-lab/dev-tracker/issues/574