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

net: TCP short writes to MariaDB #26896

Closed
xirafa opened this issue Aug 9, 2018 · 5 comments
Closed

net: TCP short writes to MariaDB #26896

xirafa opened this issue Aug 9, 2018 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@xirafa
Copy link

xirafa commented Aug 9, 2018

What version of Go are you using (go version)?

go version go1.10.3 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Aviad\AppData\Local\go-build
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\Aviad\go
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\Aviad\AppData\Local\Temp\go-build035776318=/tmp/go-build -gno-record-gcc-switches

What did you do?

  1. Create a TCP socket to mariadb db server.
  2. Wrap the socket with bufio.Writer
  3. Write() and Flush() to the buffered writer.

What did you expect to see?

Either bufio.Writer should keep writing until everything is written, or provide means to recover from the error and retry.

What did you see instead?

Got an ErrShortWrite from Flush(). Cannot recover from this error.

bufio.Writer: https://github.com/golang/go/blame/master/src/bufio/bufio.go#L577

This may be an issue with 10.x - happened after upgrade.

@bradfitz
Copy link
Contributor

bradfitz commented Aug 9, 2018

bufio is not and should not be in the business of "retrying". If your underlying Writer fails, game over.

The real question is why you're getting short writes to TCP connections on Windows.

How can we reproduce this? Do you have code?

@bradfitz bradfitz added this to the Unplanned milestone Aug 9, 2018
@bradfitz bradfitz added OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 9, 2018
@bradfitz bradfitz changed the title bufio.Writer cannot recover from short write (to TCP connections on Windows) net: TCP short writes to MariaDB Aug 9, 2018
@xirafa
Copy link
Author

xirafa commented Aug 9, 2018

Reproduction is not trivial. Perhaps the issue is with localhost TCP connections. It may take some time to prepare a clean reproduction.

What is the rationale for making short writes a sticky error? I would guess many io.Writer implementations could produce short writes.

@bradfitz
Copy link
Contributor

bradfitz commented Aug 9, 2018

What is the rationale for making short writes a sticky error?

All write errors are a sticky error.

I would guess many io.Writer implementations could produce short writes.

They should not. That's an error. The io.Writer interface is not like io.Reader. A Reader permits short reads. A Writer does not.

@xirafa
Copy link
Author

xirafa commented Aug 11, 2018

After more investigations, it appears that the cause for this error was concurrent writes to bufio.Writer.
If possible, it would be nice to detect this situation and report it using a different error.

@bradfitz
Copy link
Contributor

We do have tools to detect such cases:

https://blog.golang.org/race-detector
https://golang.org/doc/articles/race_detector.html

I'll close this. Thanks for the update.

@golang golang locked and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants