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

Use request timeout for git service rpc #20689

Merged
merged 4 commits into from
Aug 6, 2022

Conversation

parnic
Copy link
Contributor

@parnic parnic commented Aug 5, 2022

This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes #20680

This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes go-gitea#20680
@parnic
Copy link
Contributor Author

parnic commented Aug 5, 2022

@wxiaoguang let me know if this is what you had in mind. I think this is going to end up with a game of whack-a-mole trying to identify which Run commands need to be using this.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 5, 2022
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 5, 2022
@wxiaoguang
Copy link
Contributor

FYI: I think this PR is good and clear to fix the bug, and can be backported easily.

To make the Git module code clear, in the future there can be a refactor to remove both Timeout and UseContextTimeout, and make git.Run respects the context's deadline directly, then the whack-a-mole game can be avoided.

@parnic
Copy link
Contributor Author

parnic commented Aug 6, 2022

Sounds good to me. I personally think this is an important fix for 1.17.1 since it can render people unable to push or pull when it trips.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 6, 2022
@lafriks lafriks merged commit 2778990 into go-gitea:main Aug 6, 2022
@lafriks
Copy link
Member

lafriks commented Aug 6, 2022

Please send backport

@parnic parnic deleted the use-request-context-deadline branch August 6, 2022 15:44
parnic added a commit to parnic/gitea that referenced this pull request Aug 6, 2022
This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes go-gitea#20680

Co-authored-by: Lunny Xiao <[email protected]>
wxiaoguang pushed a commit that referenced this pull request Aug 7, 2022
This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes #20680
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 8, 2022
* giteaofficial/main: (23 commits)
  [skip ci] Updated translations via Crowdin
  Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702)
  Rework repo buttons (go-gitea#20602)
  Switch to building with go1.19 (go-gitea#20695)
  Update maven deploy docs (go-gitea#20691)
  Add support for Pub packages (go-gitea#20560)
  Fix typo overrided -> overridden (go-gitea#20687)
  [skip ci] Updated licenses and gitignores
  Update JS dependencies, adjust eslint (go-gitea#20659)
  Background color of private list-items updated (go-gitea#20630)
  Use request timeout for git service rpc (go-gitea#20689)
  Increase default item listing size `ISSUE_PAGING_NUM` to 20 (go-gitea#20547)
  [skip ci] Updated translations via Crowdin
  Allow multiple metadata files for Maven packages (go-gitea#20674)
  docs: update comparison.zh-cn.md (go-gitea#20656)
  Add a notice for breaking check in upgrade document (go-gitea#20682)
  move handleGlobalEnterQuickSubmit into a separate file to avoid cycle-import (go-gitea#20679)
  Use correct field name. (go-gitea#20675)
  [skip ci] Updated translations via Crowdin
  Use correct context to get package content (go-gitea#20673)
  ...

# Conflicts:
#	templates/repo/wiki/view.tmpl
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
This enables git.Command's Run to optionally use the given context directly so its deadline will be respected. Otherwise, it falls back to the previous behavior of using the supplied timeout or a default timeout value of 360 seconds.

repo's serviceRPC() calls now use the context's deadline (which is unset/unlimited) instead of the default 6-minute timeout. This means that large repo clones will no longer arbitrarily time out on the upload-pack step, and pushes can take longer than 6 minutes on the receive-pack step.

Fixes go-gitea#20680

Co-authored-by: Lunny Xiao <[email protected]>
@wxiaoguang wxiaoguang added the backport/done All backports for this PR have been created label Aug 11, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 17, 2022
 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath zeripath mentioned this pull request Aug 17, 2022
techknowlogick added a commit that referenced this pull request Aug 18, 2022
* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (#20831) (#20832)
* FEATURES
  * Add support for NuGet API keys (#20721) (#20734)
* ENHANCEMENTS
  * Display project in issue list (#20583)
  * Add disable download source configuration (#20548) (#20579)
* BUGFIXES
  * Use the total issue count for UI (#20785) (#20827)
  * Add proxy host into allow list (#20798) (#20819)
  * Add missing translation for queue flush workers (#20791) (#20792)
  * Improve comment header for mobile (#20781) (#20789)
  * Fix git.Init for doctor sub-command (#20782) (#20783)
  * Check webhooks slice length before calling xorm (#20642) (#20768)
  * Remove manual rollback for failed generated repositories (#20639) (#20762)
  * Use correct field name in npm template (#20675) (#20760)
  * Keep download count on Container tag overwrite (#20728) (#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (#20702) (#20707)
  * Use request timeout for git service rpc (#20689) (#20693)
  * Send correct NuGet status codes (#20647) (#20677)
  * Use correct context to get package content (#20673) (#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (#20584) (#20663)
  * Add default commit messages to PR for squash merge (#20618) (#20645)
  * Fix package upload for files >32mb (#20622) (#20635)
  * Fix the new-line copy-paste for rendered code (#20612)
  * Clean up and fix clone button script (#20415 & #20600) (#20599)
  *  Fix default merge style (#20564) (#20565)
  * Add repository condition for issue count (#20454) (#20496)
* MISC
  * Make branch icon stand out more (#20726) (#20774)
  * Fix loading button with invalid form (#20754) (#20759)
  * Add username check to doctor (#20140) (#20671)
  * Enable Wire 2 for Internal SSH Server (#20616) (#20617)
  *  Fix SecToTime edge-cases (#20610) (#20611)

Signed-off-by: Andrew Thornton <[email protected]>

* Apply suggestions from code review

Co-authored-by: John Olheiser <[email protected]>

* Update CHANGELOG.md

Co-authored-by: delvh <[email protected]>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <[email protected]>

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: John Olheiser <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
jolheiser pushed a commit to jolheiser/gitea that referenced this pull request Aug 22, 2022
* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <[email protected]>

* Apply suggestions from code review

Co-authored-by: John Olheiser <[email protected]>

* Update CHANGELOG.md

Co-authored-by: delvh <[email protected]>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <[email protected]>

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: John Olheiser <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
lunny added a commit that referenced this pull request Aug 23, 2022
* Changelog 1.17.1 (#20833)

* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (#20831) (#20832)
* FEATURES
  * Add support for NuGet API keys (#20721) (#20734)
* ENHANCEMENTS
  * Display project in issue list (#20583)
  * Add disable download source configuration (#20548) (#20579)
* BUGFIXES
  * Use the total issue count for UI (#20785) (#20827)
  * Add proxy host into allow list (#20798) (#20819)
  * Add missing translation for queue flush workers (#20791) (#20792)
  * Improve comment header for mobile (#20781) (#20789)
  * Fix git.Init for doctor sub-command (#20782) (#20783)
  * Check webhooks slice length before calling xorm (#20642) (#20768)
  * Remove manual rollback for failed generated repositories (#20639) (#20762)
  * Use correct field name in npm template (#20675) (#20760)
  * Keep download count on Container tag overwrite (#20728) (#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (#20702) (#20707)
  * Use request timeout for git service rpc (#20689) (#20693)
  * Send correct NuGet status codes (#20647) (#20677)
  * Use correct context to get package content (#20673) (#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (#20584) (#20663)
  * Add default commit messages to PR for squash merge (#20618) (#20645)
  * Fix package upload for files >32mb (#20622) (#20635)
  * Fix the new-line copy-paste for rendered code (#20612)
  * Clean up and fix clone button script (#20415 & #20600) (#20599)
  *  Fix default merge style (#20564) (#20565)
  * Add repository condition for issue count (#20454) (#20496)
* MISC
  * Make branch icon stand out more (#20726) (#20774)
  * Fix loading button with invalid form (#20754) (#20759)
  * Add username check to doctor (#20140) (#20671)
  * Enable Wire 2 for Internal SSH Server (#20616) (#20617)
  *  Fix SecToTime edge-cases (#20610) (#20611)

Signed-off-by: Andrew Thornton <[email protected]>

* Apply suggestions from code review

Co-authored-by: John Olheiser <[email protected]>

* Update CHANGELOG.md

Co-authored-by: delvh <[email protected]>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <[email protected]>

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: John Olheiser <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>

* update changelog

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: zeripath <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 28, 2022
* Changelog 1.17.1 (go-gitea#20833)

* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <[email protected]>

* Apply suggestions from code review

Co-authored-by: John Olheiser <[email protected]>

* Update CHANGELOG.md

Co-authored-by: delvh <[email protected]>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <[email protected]>

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: John Olheiser <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>

* update changelog

Signed-off-by: Andrew Thornton <[email protected]>
Co-authored-by: zeripath <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: delvh <[email protected]>
Co-authored-by: Lauris BH <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[1.17.0] Context deadline exceeded (timeout) cloning large repo
6 participants