Skip to content

Commit dbd6fe1

Browse files
committed
Option to delay conflict checking of old pull requests until page view
[repository.pull-request] CHECK_ONLY_LAST_UPDATED_DAYS specifies the number of days in which the PR must have been last updated for conflict checking to happen immediately when the base branch is updated. The default value of -1 behaves exactly as before with no delay. With 0 all conflict checking will be delayed until page view. We have found that 1 day works well in practice. This mostly eliminates the "Merge conflict checking is in progress. Try again in few moments." message when conflict checking is slow for big repositories and many pull requests. PRs that are actively being worked on will be checked immediately, while for others are likely to actually get checked in the next few seconds. Even better would be to auto-update the page as GitHub does, but this relatively simple change effectively solved the problem for us.
1 parent f05d9c9 commit dbd6fe1

File tree

6 files changed

+91
-10
lines changed

6 files changed

+91
-10
lines changed

custom/conf/app.example.ini

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,11 @@ LEVEL = Info
10871087
;;
10881088
;; Retarget child pull requests to the parent pull request branch target on merge of parent pull request. It only works on merged PRs where the head and base branch target the same repo.
10891089
;RETARGET_CHILDREN_ON_MERGE = true
1090+
;;
1091+
;; Delay conflict checking until page view or API access, for pull requests that have not been updated in the specified number of days.
1092+
;; The default `-1` means never delay. With `1` day, pull requests under active development will be checked quickly without undue server
1093+
;; load for old pull requests.
1094+
;CHECK_ONLY_LAST_UPDATED_DAYS = -1
10901095

10911096
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
10921097
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

modules/setting/repository.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ var (
8686
AddCoCommitterTrailers bool
8787
TestConflictingPatchesWithGitApply bool
8888
RetargetChildrenOnMerge bool
89+
CheckOnlyLastUpdatedDays int
8990
} `ini:"repository.pull-request"`
9091

9192
// Issue Setting
@@ -211,6 +212,7 @@ var (
211212
AddCoCommitterTrailers bool
212213
TestConflictingPatchesWithGitApply bool
213214
RetargetChildrenOnMerge bool
215+
CheckOnlyLastUpdatedDays int
214216
}{
215217
WorkInProgressPrefixes: []string{"WIP:", "[WIP]"},
216218
// Same as GitHub. See
@@ -226,6 +228,7 @@ var (
226228
PopulateSquashCommentWithCommitMessages: false,
227229
AddCoCommitterTrailers: true,
228230
RetargetChildrenOnMerge: true,
231+
CheckOnlyLastUpdatedDays: -1,
229232
},
230233

231234
// Issue settings

routers/api/v1/repo/pull.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,10 @@ func GetPullRequest(ctx *context.APIContext) {
205205
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
206206
return
207207
}
208+
209+
// Consider API access a view for delayed checking.
210+
pull_service.AddToTaskQueueOnView(ctx, pr)
211+
208212
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(ctx, pr, ctx.Doer))
209213
}
210214

@@ -290,6 +294,10 @@ func GetPullRequestByBaseHead(ctx *context.APIContext) {
290294
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
291295
return
292296
}
297+
298+
// Consider API access a view for delayed checking.
299+
pull_service.AddToTaskQueueOnView(ctx, pr)
300+
293301
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(ctx, pr, ctx.Doer))
294302
}
295303

routers/web/repo/issue.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,6 +1824,8 @@ func ViewIssue(ctx *context.Context) {
18241824
allowMerge := false
18251825
canWriteToHeadRepo := false
18261826

1827+
pull_service.AddToTaskQueueOnView(ctx, pull)
1828+
18271829
if ctx.IsSigned {
18281830
if err := pull.LoadHeadRepo(ctx); err != nil {
18291831
log.Error("LoadHeadRepo: %v", err)

services/pull/check.go

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"strconv"
1212
"strings"
13+
"time"
1314

1415
"code.gitea.io/gitea/models"
1516
"code.gitea.io/gitea/models/db"
@@ -26,6 +27,7 @@ import (
2627
"code.gitea.io/gitea/modules/log"
2728
"code.gitea.io/gitea/modules/process"
2829
"code.gitea.io/gitea/modules/queue"
30+
"code.gitea.io/gitea/modules/setting"
2931
"code.gitea.io/gitea/modules/timeutil"
3032
asymkey_service "code.gitea.io/gitea/services/asymkey"
3133
notify_service "code.gitea.io/gitea/services/notify"
@@ -44,21 +46,85 @@ var (
4446
ErrDependenciesLeft = errors.New("is blocked by an open dependency")
4547
)
4648

47-
// AddToTaskQueue adds itself to pull request test task queue.
48-
func AddToTaskQueue(ctx context.Context, pr *issues_model.PullRequest) {
49+
// Change the pull request status to checking.
50+
func setStatusChecking(ctx context.Context, pr *issues_model.PullRequest) bool {
4951
pr.Status = issues_model.PullRequestStatusChecking
5052
err := pr.UpdateColsIfNotMerged(ctx, "status")
5153
if err != nil {
52-
log.Error("AddToTaskQueue(%-v).UpdateCols.(add to queue): %v", pr, err)
53-
return
54+
log.Error("setStatusChecking(%-v).UpdateCols.(add to queue): %v", pr, err)
55+
return false
5456
}
57+
return true
58+
}
59+
60+
// Add pull request in the actual queue.
61+
func addToTaskQueue(pr *issues_model.PullRequest) {
5562
log.Trace("Adding %-v to the test pull requests queue", pr)
56-
err = prPatchCheckerQueue.Push(strconv.FormatInt(pr.ID, 10))
63+
err := prPatchCheckerQueue.Push(strconv.FormatInt(pr.ID, 10))
5764
if err != nil && err != queue.ErrAlreadyInQueue {
5865
log.Error("Error adding %-v to the test pull requests queue: %v", pr, err)
5966
}
6067
}
6168

69+
// Test if check should be delayed.
70+
func shouldDelayCheck(ctx context.Context, pr *issues_model.PullRequest) bool {
71+
if setting.Repository.PullRequest.CheckOnlyLastUpdatedDays >= 0 {
72+
if err := pr.LoadIssue(ctx); err != nil {
73+
return true
74+
}
75+
delay := time.Hour * 24 * time.Duration(setting.Repository.PullRequest.CheckOnlyLastUpdatedDays)
76+
if pr.Issue.UpdatedUnix.AddDuration(delay) < timeutil.TimeStampNow() {
77+
log.Trace("Delaying %-v patch checking because it was not updated recently", pr)
78+
return true
79+
}
80+
}
81+
return false
82+
}
83+
84+
// When base branch is updated.
85+
func AddToTaskQueueOnBaseUpdate(ctx context.Context, pr *issues_model.PullRequest) {
86+
if !setStatusChecking(ctx, pr) {
87+
return
88+
}
89+
if shouldDelayCheck(ctx, pr) {
90+
return
91+
}
92+
addToTaskQueue(pr)
93+
}
94+
95+
// When pull request is viewed by a user or through the API.
96+
func AddToTaskQueueOnView(ctx context.Context, pr *issues_model.PullRequest) {
97+
if setting.Repository.PullRequest.CheckOnlyLastUpdatedDays >= 0 &&
98+
pr.Status == issues_model.PullRequestStatusChecking {
99+
addToTaskQueue(pr)
100+
}
101+
}
102+
103+
// When Gitea restarts.
104+
func AddToTaskQueueOnInit(ctx context.Context, prID int64) {
105+
if setting.Repository.PullRequest.CheckOnlyLastUpdatedDays >= 0 {
106+
pr, err := issues_model.GetPullRequestByID(ctx, prID)
107+
if err != nil {
108+
return
109+
}
110+
if shouldDelayCheck(ctx, pr) {
111+
return
112+
}
113+
}
114+
115+
log.Trace("Adding PR[%d] to the pull requests patch checking queue", prID)
116+
if err := prPatchCheckerQueue.Push(strconv.FormatInt(prID, 10)); err != nil {
117+
log.Error("Error adding PR[%d] to the pull requests patch checking queue %v", prID, err)
118+
}
119+
}
120+
121+
// When pull request must be checked immediately without delay.
122+
func AddToTaskQueue(ctx context.Context, pr *issues_model.PullRequest) {
123+
if setStatusChecking(ctx, pr) {
124+
addToTaskQueue(pr)
125+
}
126+
}
127+
62128
type MergeCheckType int
63129

64130
const (
@@ -317,10 +383,7 @@ func InitializePullRequests(ctx context.Context) {
317383
case <-ctx.Done():
318384
return
319385
default:
320-
log.Trace("Adding PR[%d] to the pull requests patch checking queue", prID)
321-
if err := prPatchCheckerQueue.Push(strconv.FormatInt(prID, 10)); err != nil {
322-
log.Error("Error adding PR[%d] to the pull requests patch checking queue %v", prID, err)
323-
}
386+
AddToTaskQueueOnInit(ctx, prID)
324387
}
325388
}
326389
}

services/pull/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
431431
log.Error("UpdateCommitDivergence: %v", err)
432432
}
433433
}
434-
AddToTaskQueue(ctx, pr)
434+
AddToTaskQueueOnBaseUpdate(ctx, pr)
435435
}
436436
})
437437
}

0 commit comments

Comments
 (0)