Skip to content

WIP: Move migration git clone to its own goroutine#5949

Closed
zeripath wants to merge 12 commits into
go-gitea:masterfrom
zeripath:issue-3770-put-clone-in-goroutine
Closed

WIP: Move migration git clone to its own goroutine#5949
zeripath wants to merge 12 commits into
go-gitea:masterfrom
zeripath:issue-3770-put-clone-in-goroutine

Conversation

@zeripath

@zeripath zeripath commented Feb 3, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

The current code places the git clone in the POST goroutine, blocking that goroutine until it is finished. This PR asynchronises this, placing the clone within its own goroutine. Fix #3770. Fix #1249.

lafriks
lafriks reviewed Feb 3, 2019
View reviewed changes
Comment thread templates/user/dashboard/feeds.tmpl Outdated
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 3, 2019
@zeripath

zeripath commented Feb 3, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author
  • migration goroutine needs panic protection with a recover.
  • need to think about how migration failures are notified. I currently attach these in an action to the repo, but these should be deleted when the repo is... I think there must be a better way of informing errors etc. Turns out this is not possible.. Therefore if the migration fails I will keep the repository around
  • need to fix tests
  • rebase on head fix conflicts

@zeripath zeripath force-pushed the issue-3770-put-clone-in-goroutine branch from e20736d to e9dd53b Compare February 4, 2019 18:03
@zeripath
Move migration clone to goroutine
938026f 
The current code places the git clone in the POST goroutine, blocking
that goroutine until it is finished. This PR asynchronises this,
placing the clone within its own goroutine. Fix go-gitea#3770
@zeripath zeripath force-pushed the issue-3770-put-clone-in-goroutine branch from e9dd53b to 938026f Compare February 4, 2019 18:05
@codecov-io

codecov-io commented Feb 4, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5949 into master will increase coverage by 0.04%.
The diff coverage is 48.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5949      +/-   ##
=========================================
+ Coverage   38.86%   38.9%   +0.04%     
=========================================
  Files         345     346       +1     
  Lines       49510   49670     +160     
=========================================
+ Hits        19241   19324      +83     
- Misses      27486   27553      +67     
- Partials     2783    2793      +10
Impacted Files Coverage Δ
models/action.go 60.98% <ø> (ø) ⬆️
modules/templates/helper.go 48.95% <0%> (ø) ⬆️
models/repo.go 46.83% <28.57%> (-0.45%) ⬇️
models/repo_migrate.go 47.98% <47.98%> (ø)
routers/repo/repo.go 21.4% <53.84%> (+0.58%) ⬆️
routers/api/v1/repo/repo.go 56.79% <83.33%> (+0.15%) ⬆️
modules/util/sanitize.go 63.15% <0%> (+63.15%) ⬆️

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 11e3166...986060e. Read the comment docs.

@zeripath

zeripath commented Feb 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ok, this is now working. In terms of things that could be done:

  • At present I just create an empty repository and archive it as a placeholder for the migration. It may be possible to set something in the repository to show it is in migration?
  • I haven't checked what happens should you try to delete this repository whilst it is in migration. That may be horrible... It was horrible - this is now fixed.
  • It may actually make sense to try to recover as much as possible from a failed migration rather than just delete failed clones.

@zeripath zeripath mentioned this pull request Feb 17, 2019
Closed
7 tasks
@jolheiser jolheiser mentioned this pull request Mar 1, 2019
Merged
@zeripath

zeripath commented Mar 15, 2019

Copy link
Copy Markdown
Contributor Author

Closing this PR in preference to lunny's

@zeripath zeripath closed this Mar 15, 2019
@zeripath zeripath deleted the issue-3770-put-clone-in-goroutine branch April 22, 2019 20:22
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lafriks lafriks lafriks left review comments

Assignees

No one assigned

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Page not reacting while long-running migration/mirror initialization Repositories are broken after unsuccessful clone/mirror creation

4 participants

@zeripath @codecov-io @lafriks @GiteaBot