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

Remove use MakeAssigneeList in webhooks to fix deadlock #6102

Merged

Conversation

@zeripath
Copy link
Contributor

commented Feb 17, 2019

The webhooks currently use a call to MakeAssigneeList to get the assignees of an issue - however, this causes an SQL deadlock due to use of models.x whilst in a transaction. This call is unnecessary as the assignees are actually provided in the api.PullRequest.

Fix #6088

Signed-off-by: Andrew Thornton art27@cantab.net


Ceterum autem censeo x exempla monstrabit esse delendam.

Remove use MakeAssigneeList
Signed-off-by: Andrew Thornton <art27@cantab.net>

@zeripath zeripath added this to the 1.8.0 milestone Feb 17, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Feb 17, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@8d3bb86). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6102   +/-   ##
=========================================
  Coverage          ?   38.86%           
=========================================
  Files             ?      345           
  Lines             ?    49510           
  Branches          ?        0           
=========================================
  Hits              ?    19241           
  Misses            ?    27486           
  Partials          ?     2783
Impacted Files Coverage Δ
models/webhook_dingtalk.go 0% <0%> (ø)
models/webhook_discord.go 1.39% <0%> (ø)
models/webhook_slack.go 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d3bb86...ff3b664. Read the comment docs.

@zeripath zeripath changed the title Remove use MakeAssigneeList in webhooks Remove use MakeAssigneeList in webhooks to fix deadlock Feb 17, 2019

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Feb 17, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Feb 17, 2019

@zeripath zeripath merged commit 11e3166 into go-gitea:master Feb 17, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@zeripath zeripath deleted the zeripath:issue-6088-deadlock-in-webhook-generation branch Feb 17, 2019

zeripath added a commit to zeripath/gitea that referenced this pull request Feb 17, 2019

Fix deadlock in webhook PullRequest (go-gitea#6102)
Signed-off-by: Andrew Thornton <art27@cantab.net>

techknowlogick added a commit that referenced this pull request Feb 18, 2019

Fix deadlock in webhook PullRequest (#6102) (#6104)
Signed-off-by: Andrew Thornton <art27@cantab.net>

Mikescher added a commit to Mikescher/gitea that referenced this pull request Mar 20, 2019

Fix deadlock in webhook PullRequest (go-gitea#6102)
Signed-off-by: Andrew Thornton <art27@cantab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.