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: deadlock on transaction stmt context cancel [1.15 backport] #42884

Closed
gopherbot opened this issue Nov 30, 2020 · 8 comments
Closed

database/sql: deadlock on transaction stmt context cancel [1.15 backport] #42884

gopherbot opened this issue Nov 30, 2020 · 8 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.15.11

Comments

@gopherbot
Copy link
Contributor

gopherbot commented Nov 30, 2020 •
edited by dmitshur
Loading

@pjm0616 requested issue #40985 (fixed by d4c1ad8 and d8044a6 in Go 1.16) to be considered for backport to the next 1.15 minor release.

@gopherbot @odeke-em Please consider this for backport to 1.15 and 1.14.
This regression, introduced in 1.14.6 and 1.15(by #34775), has a serious impact(deadlock) and no workarounds, other than patching Go manually.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 30, 2020
@gopherbot gopherbot mentioned this issue Nov 30, 2020
Closed
@gopherbot gopherbot added this to the Go1.15.6 milestone Nov 30, 2020
@cagedmantis cagedmantis modified the milestones: Go1.15.6, Go1.15.7 Dec 3, 2020
@cagedmantis
Copy link
Contributor

Approved as this is a serious issue without a known workaround.

@cagedmantis cagedmantis added the CherryPickApproved Used during the release process for point releases label Jan 14, 2021
@cagedmantis
Copy link
Contributor

Please remember to create the cherry-pick CL.

@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jan 14, 2021
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/284513 mentions this issue: [release-branch.go1.15] database/sql: fix tx stmt deadlock when rollback

@odeke-em
Copy link
Member

Roger that, I've mailed a CL.

@dmitshur dmitshur modified the milestones: Go1.15.7, Go1.15.8 Jan 19, 2021
@cagedmantis
Copy link
Contributor

Would @kardianos be willing to review this since they approved the original CL?

@cagedmantis cagedmantis modified the milestones: Go1.15.8, Go1.15.9 Feb 4, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Mar 4, 2021

@odeke-em The CL has some outstanding comments, can you please take a look?

@odeke-em
Copy link
Member

odeke-em commented Mar 5, 2021 •
edited
Loading

Roger that, let me take a look, I don't think I had received Gerrit emails for them. Thank you @dmitshur!

@toothrot toothrot modified the milestones: Go1.15.9, Go1.15.10 Mar 10, 2021
@cagedmantis cagedmantis modified the milestones: Go1.15.10, Go1.15.11 Mar 11, 2021
@gopherbot
Copy link
Contributor Author

Closed by merging c77418f to release-branch.go1.15.

gopherbot pushed a commit that referenced this issue Mar 30, 2021
@odeke-em @dmitshur
[release-branch.go1.15] database/sql: fix tx stmt deadlock when rollback
c77418f 
Tx acquires tx.closemu W-lock and then acquires stmt.closemu.W-lock
to fully close the transaction and associated prepared statement.
Stmt query and execution run in reverse ways - acquires
stmt.closemu.R-lock and then acquires tx.closemu.R-lock to grab tx
connection, which may cause deadlock.

Prevent the lock is held around tx.closePrepared to ensure no
deadlock happens.

Includes a test fix from CL 266097.
Fixes #42884
Updates #40985
Updates #42259

Change-Id: Id52737660ada3cebdfff6efc23366cdc3224b8e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/250178
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Daniel Theophanes <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
(cherry picked from commit d4c1ad8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/284513
Reviewed-by: Dmitri Shuralyov <[email protected]>
@knz knz mentioned this issue Apr 19, 2021
Closed
@rickystewart rickystewart mentioned this issue Apr 19, 2021
Merged
11 tasks
rickystewart added a commit to rickystewart/cockroach that referenced this issue Apr 19, 2021
@rickystewart
build: upgrade go to 1.15.11
3d03033 
Pick up fixes to the following Go bugs:

* golang/go#45076
* golang/go#45187
* golang/go#42884

* [ ] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [x] Adjust `GO_VERSION` in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
  and ask the Developer Infrastructure team to deploy new images.

Resolves cockroachdb#63836.

Release note (general change): Upgrade the CRDB binary to Go 1.15.10
craig bot pushed a commit to cockroachdb/cockroach that referenced this issue Apr 19, 2021
@knz @rickystewart
Merge #63785 #63787 #63839 #63853 #63857
0609f89 
63785: sql: use catalog.Mutation, catalog.Column and catalog.Index where possible r=postamar a=postamar

    sql: use catalog.Mutation where possible
    
    This commit is a refactor which introduces catalog.Mutation wherever
    possible, in an effort to avoid using the descpb.DescriptorMutation
    type directly.
    
    Release note: None


    sql: use catalog.Column and catalog.Index where possible
    
    This commit is a refactor which introduces the catalog.Column and
    catalog.Index interfaces wherever possible. These had recently been
    added in an effort to avoid using the descpb.ColumnDescriptor and
    descpb.IndexDescriptor types directly. This commit generalizes their
    usage throughout the sql packages.
    
    Fixes #63755.
    
    Release note: None



63787: roachtest: added hibernate ignorelist for 21.1 r=rafiss a=mnovelodou

Previously, some random issues with hibernate were afecting roachtest
This was inadequate because they are false failures
To address this, this patch set these tests in ignore list

Release note: none

63839: cli: bump the cobra dependency and add autocompletion for fish r=stevendanna a=knz

Fixes  #63838 . 
Fixes  #50187.

This updates the cobra dep which hadn't been updated since 2017.

Also picks up the new support for fish autocompletions.

Release note (cli change): Certain errors caused by invalid
command-line arguments are now printed on the process' standard
error stream, instead of standard output.

Release note (cli change): The `cockroach gen autocomplete` command
has been updated and can now produce autocompletion definitions
for the `fish` shell.

63853: build: upgrade go to 1.15.11 r=rail a=rickystewart

Pick up fixes to the following Go bugs:

* golang/go#45076
* golang/go#45187
* golang/go#42884

* [ ] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [x] Adjust `GO_VERSION` in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
  and ask the Developer Infrastructure team to deploy new images.

Resolves #63836.

Release note (general change): Upgrade the CRDB binary to Go 1.15.10

63857: changefeedccl: Make `kafka_sink_config` a valid option. r=stevendanna a=miretskiy

Add `kafka_sink_config` to the list of allowed changefeed options.

Release Notes: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: MiguelNovelo <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>
rickystewart added a commit to rickystewart/cockroach that referenced this issue Apr 19, 2021
@rickystewart
build: upgrade go to 1.15.11
564b086 
Pick up fixes to the following Go bugs:

* golang/go#45076
* golang/go#45187
* golang/go#42884

* [ ] Adjust the Pebble tests to run in new version.
* [x] Adjust version in Docker image ([source](./builder/Dockerfile)).
* [x] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [x] Bump the version in `WORKSPACE` under `go_register_toolchains`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases).
* [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.
* [x] Adjust `GO_VERSION` in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh))
  and ask the Developer Infrastructure team to deploy new images.

Resolves cockroachdb#63836.

Release note (general change): Upgrade the CRDB binary to Go 1.15.10
@rickystewart rickystewart mentioned this issue Apr 19, 2021
Merged
11 tasks
jseldess pushed a commit to cockroachdb/docs that referenced this issue May 4, 2021
Update Go version requirement for building 21.1 from source
094d7cd 
Technically, the minimum required is 1.15.3:
https://github.com/cockroachdb/cockroach/blob/release-21.1/build/go-version-check.sh#L10

However, 1.15.11 avoids these Go bugs:
golang/go#45076
golang/go#45187
golang/go#42884

So to prevent users from running into those, we listing
1.15.11 as the minimum required version, followed by
a note that you can use IGNORE_GOVERS=1 to try building
with older versions.

Fixes #10468
Fixes #9081
@jseldess jseldess mentioned this issue May 4, 2021
Merged
@golang golang locked and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.15.11
Development

No branches or pull requests
5 participants
@toothrot @cagedmantis @dmitshur @odeke-em @gopherbot