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.

Sign up for GitHub

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

Jump to bottom

database/sql: go1.4 introduces locking issue in gosqlite #9380

Closed
vbatts opened this issue Dec 18, 2014 · 5 comments
Closed

database/sql: go1.4 introduces locking issue in gosqlite #9380

vbatts opened this issue Dec 18, 2014 · 5 comments
Labels
FrozenDueToAge

Comments

@vbatts
Copy link
Contributor

vbatts commented Dec 18, 2014

Still working on isolating the exact issue, but have bisected the culprit to this commit 6fb6f4e (or hg commit a701ced4b6da)

In docker, the graphdb uses code.google.com/p/gosqlite/sqlite3. If you build using the above commit to go1.4 release, the first docker run will produce FATA[0000] Error response from daemon: The database file is locked: database is locked
Here is the callgrind image of the graphdb callgraph:

Related issue moby/moby#9615 (comment)
presently this is a blocker for docker to use go1.4
@vbatts
Copy link
Contributor Author

vbatts commented Dec 18, 2014

@bradfitz thoughts?

@mattn
Copy link
Member

mattn commented Dec 18, 2014

If you try to replace sqlite3 driver, we can isolate the problem whether caused in Go or driver, I guess.
Could you try to replace code.google.com/p/gosqlite/sqlite3 to github.com/mattn/go-sqlite3 ? This should work as same as gosqlite.

https://github.com/docker/docker/blob/v1.4.1/pkg/graphdb/conn_sqlite3.go#L8

@unclejack
Copy link
Contributor

@mattn I've tried this and I was getting errors even earlier. I'll try this again and update this comment with the output.

With Go 1.4 and mattn/go-sqlite3, make test-integration-cli fails right away:

Sending build context to Docker daemon 
Step 0 : FROM scratch
 ---> 511136ea3c5a
Step 1 : MAINTAINER Jérôme Petazzoni <jerome@docker.com>
 ---> Running in 7086b4880ea1
 ---> c6bff68e2e73
Removing intermediate container 7086b4880ea1
Step 2 : ADD rootfs.tar /
 ---> cf8da6d4dfd1
Removing intermediate container cfebe43d079c
Step 3 : CMD /bin/sh
 ---> Running in 0c6b50384b85
 ---> 347974cadc4d
Removing intermediate container 0c6b50384b85
Successfully built 347974cadc4d
+ go test -test.timeout=30m github.com/docker/docker/integration-cli
[PASSED]: container REST API - check GET json/all=1
--- FAIL: TestContainerApiGetExport (0.02s)
        docker_api_containers_test.go:59: Error on container creation: exit status 1, output: "time=\"2014-12-18T17:59:10Z\" level=\"fatal\" msg=\"Error response from daemon: cannot start a transaction within a transaction\" \n"
--- FAIL: TestContainerApiGetChanges (5.02s)
        docker_api_containers_test.go:95: Error on container creation: exit status 1, output: "time=\"2014-12-18T17:59:15Z\" level=\"fatal\" msg=\"Error response from daemon: database is locked\" \n"
--- FAIL: TestContainerApiStartVolumeBinds (0.08s)
        docker_api_containers_test.go:136: received status != 200 OK: 500 Internal Server Error
--- FAIL: TestContainerApiStartVolumesFrom (0.09s)
        docker_api_containers_test.go:169: time="2014-12-18T17:59:15Z" level="fatal" msg="Error response from daemon: cannot start a transaction within a transaction"

@LK4D4 LK4D4 mentioned this issue Dec 18, 2014
Merged
@johto
Copy link
Contributor

johto commented Dec 18, 2014

I had a quick look at this, and I didn't bother to see if I'm looking at the code that causes the problem in this case, but this is absolutely broken. It's assuming both that subsequent executions of *sql.DB.Exec() operate on the same connection, and that nobody else can operate on the same connection between the two invocations. Both assumptions are wrong.

The answer here might just be that the code was relying on undefined behaviour before and just happened to not break pre-1.4. But I'm personally not going to spend any more time on this issue until there's a reproducible, small test case.

@vbatts
Copy link
Contributor Author

vbatts commented Dec 19, 2014

@johto thanks for the comment. @unclejack has fixed that now and the issue went away.

@vbatts vbatts closed this as completed Dec 19, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mattn @vbatts @johto @unclejack @gopherbot