Move all push update operations to a queue #10133

lunny · 2020-02-04T02:26:59Z

More and more operations will be fired when commits pushed, so move it to a queue is a better way. It will let git clients receive reponse quicker and also, we could add more operations when pushing. i.e. cache big repos, language calculations and etc.

services/repository/push.go

guillep2k · 2020-02-04T18:09:03Z

services/repository/push.go

+}
+
+func initPushQueue() error {
+	pushQueue = queue.CreateQueue("push_update", handle, []*PushUpdateOptions{}).(queue.Queue)


There are defaults that maybe we should want to change, here. Like BATCH_LENGTH = 20.

This could be changed by user.

What I mean is that it might be useful to tailor these defaults. We're currently using the same queue defaults for all queues, but maybe git-centered activity queues could benefit from defaults different than e-mail delivery activity queues, for example.

If this is wanted specific defaults overrides would be set:

gitea/modules/setting/queue.go

Lines 87 to 163 in e4b3f35

// NewQueueService sets up the default settings for Queues

// This is exported for tests to be able to use the queue

func NewQueueService() {

sec := Cfg.Section("queue")

Queue.DataDir = sec.Key("DATADIR").MustString("queues/")

if !filepath.IsAbs(Queue.DataDir) {

Queue.DataDir = filepath.Join(AppDataPath, Queue.DataDir)

}

Queue.Length = sec.Key("LENGTH").MustInt(20)

Queue.BatchLength = sec.Key("BATCH_LENGTH").MustInt(20)

Queue.ConnectionString = sec.Key("CONN_STR").MustString(path.Join(AppDataPath, ""))

Queue.Type = sec.Key("TYPE").MustString("persistable-channel")

Queue.Network, Queue.Addresses, Queue.Password, Queue.DBIndex, _ = ParseQueueConnStr(Queue.ConnectionString)

Queue.WrapIfNecessary = sec.Key("WRAP_IF_NECESSARY").MustBool(true)

Queue.MaxAttempts = sec.Key("MAX_ATTEMPTS").MustInt(10)

Queue.Timeout = sec.Key("TIMEOUT").MustDuration(GracefulHammerTime + 30*time.Second)

Queue.Workers = sec.Key("WORKERS").MustInt(1)

Queue.MaxWorkers = sec.Key("MAX_WORKERS").MustInt(10)

Queue.BlockTimeout = sec.Key("BLOCK_TIMEOUT").MustDuration(1 * time.Second)

Queue.BoostTimeout = sec.Key("BOOST_TIMEOUT").MustDuration(5 * time.Minute)

Queue.BoostWorkers = sec.Key("BOOST_WORKERS").MustInt(5)

Queue.QueueName = sec.Key("QUEUE_NAME").MustString("_queue")

Queue.SetName = sec.Key("SET_NAME").MustString("")

// Now handle the old issue_indexer configuration

section := Cfg.Section("queue.issue_indexer")

sectionMap := map[string]bool{}

for _, key := range section.Keys() {

sectionMap[key.Name()] = true

}

if _, ok := sectionMap["TYPE"]; !ok {

switch Indexer.IssueQueueType {

case LevelQueueType:

_, _ = section.NewKey("TYPE", "level")

case ChannelQueueType:

_, _ = section.NewKey("TYPE", "persistable-channel")

case RedisQueueType:

_, _ = section.NewKey("TYPE", "redis")

default:

log.Fatal("Unsupported indexer queue type: %v",

Indexer.IssueQueueType)

}

}

if _, ok := sectionMap["LENGTH"]; !ok {

_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Indexer.UpdateQueueLength))

}

if _, ok := sectionMap["BATCH_LENGTH"]; !ok {

_, _ = section.NewKey("BATCH_LENGTH", fmt.Sprintf("%d", Indexer.IssueQueueBatchNumber))

}

if _, ok := sectionMap["DATADIR"]; !ok {

_, _ = section.NewKey("DATADIR", Indexer.IssueQueueDir)

}

if _, ok := sectionMap["CONN_STR"]; !ok {

_, _ = section.NewKey("CONN_STR", Indexer.IssueQueueConnStr)

}

// Handle the old mailer configuration

section = Cfg.Section("queue.mailer")

sectionMap = map[string]bool{}

for _, key := range section.Keys() {

sectionMap[key.Name()] = true

}

if _, ok := sectionMap["LENGTH"]; !ok {

_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Cfg.Section("mailer").Key("SEND_BUFFER_LEN").MustInt(100)))

}

// Handle the old test pull requests configuration

// Please note this will be a unique queue

section = Cfg.Section("queue.pr_patch_checker")

sectionMap = map[string]bool{}

for _, key := range section.Keys() {

sectionMap[key.Name()] = true

}

if _, ok := sectionMap["LENGTH"]; !ok {

_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Repository.PullRequestQueueLength))

}

}

lunny · 2020-02-18T10:04:48Z

In the new commit of this PR, I added a NoQueue for easier tests.

codecov-io · 2020-02-18T10:17:12Z

Codecov Report

Merging #10133 into master will increase coverage by 0.25%.
The diff coverage is 54.3%.

@@            Coverage Diff             @@
##           master   #10133      +/-   ##
==========================================
+ Coverage   43.71%   43.96%   +0.25%     
==========================================
  Files         586      588       +2     
  Lines       81351    82339     +988     
==========================================
+ Hits        35563    36203     +640     
- Misses      41393    41703     +310     
- Partials     4395     4433      +38

Impacted Files	Coverage Δ
modules/repofiles/update.go	`36.12% <ø> (-2.91%)`	⬇️
routers/init.go	`61.05% <0%> (-2%)`	⬇️
services/repository/init.go	`100% <100%> (ø)`
routers/repo/pull.go	`28.54% <33.33%> (-0.08%)`	⬇️
modules/queue/queue.go	`28.98% <37.5%> (+4.54%)`	⬆️
routers/repo/branch.go	`55.38% <46.15%> (-0.35%)`	⬇️
services/repository/push.go	`54.66% <54.66%> (ø)`
services/repository/action.go	`61.29% <61.29%> (ø)`
routers/private/hook.go	`36.43% <66.66%> (ø)`	⬆️
modules/indexer/stats/db.go	`50% <0%> (-18.75%)`	⬇️
... and 25 more

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 9ed4aee...76317f3. Read the comment docs.

lunny · 2020-02-18T12:41:52Z

It's ready to review.

modules/queue/queue.go

guillep2k · 2020-02-19T01:07:57Z

services/repository/push.go

+}
+
+func initPushQueue() error {
+	pushQueue = queue.CreateQueue("push_update", handle, []*PushUpdateOptions{}).(queue.Queue)


What I mean is that it might be useful to tailor these defaults. We're currently using the same queue defaults for all queues, but maybe git-centered activity queues could benefit from defaults different than e-mail delivery activity queues, for example.

services/repository/push.go

6543 · 2020-04-07T15:39:35Z

can you resolve conflicts?

stale · 2020-07-18T06:58:49Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

lafriks · 2020-07-25T11:44:18Z

Conflicts

codecov-commenter · 2020-09-05T12:06:05Z

Codecov Report

Merging #10133 into master will decrease coverage by 0.01%.
The diff coverage is 60.20%.

@@            Coverage Diff             @@
##           master   #10133      +/-   ##
==========================================
- Coverage   43.18%   43.16%   -0.02%     
==========================================
  Files         651      652       +1     
  Lines       72145    72085      -60     
==========================================
- Hits        31154    31114      -40     
+ Misses      35942    35926      -16     
+ Partials     5049     5045       -4

Impacted Files	Coverage Δ
modules/repofiles/action.go	`42.15% <ø> (-11.14%)`	⬇️
modules/repofiles/update.go	`37.08% <ø> (-3.22%)`	⬇️
routers/init.go	`66.66% <0.00%> (-1.38%)`	⬇️
services/repository/push.go	`58.56% <58.56%> (ø)`
routers/private/hook.go	`29.52% <66.66%> (ø)`
routers/api/v1/repo/branch.go	`37.43% <100.00%> (-0.33%)`	⬇️
routers/repo/branch.go	`58.33% <100.00%> (-0.72%)`	⬇️
routers/repo/pull.go	`33.53% <100.00%> (-0.17%)`	⬇️
services/repository/repository.go	`58.13% <100.00%> (+2.04%)`	⬆️
modules/util/timer.go	`42.85% <0.00%> (-42.86%)`	⬇️
... and 6 more

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 910947f...8f641ea. Read the comment docs.

6543 · 2020-09-05T15:15:03Z

I'm just unsure, but there is only one removed line, do this code really work 😅 ?

If I remember correctly git push things are initialy handled by the HookPreReceive in routers/private/...

lunny · 2020-09-08T08:49:18Z

@6543 please review again.

zeripath · 2020-09-08T09:56:05Z

services/repository/push.go

+	_, err = git.NewCommand("update-server-info").RunInDir(repoPath)
+	if err != nil {
+		return fmt.Errorf("Failed to call 'git update-server-info': %v", err)
+	}


I'm concerned that this is outdated - update-server-info is only used for dumb HTTP and therefore this should be removed.

It's also running at the wrong time here. If we want to use update-server-info we should be using a git post-update hook.

then again it appears we do support the info/refs endpoint ... hmm ...

hmm old code has it too - so I'm ok with it - if this gets problematic I think this can have its own pull

👇 so LGTM from my side :)

6543

tested works

6543 · 2020-09-11T00:11:36Z

failed unrelated

lunny added the type/enhancement An improvement of existing functionality label Feb 4, 2020

lunny mentioned this pull request Feb 4, 2020

Cache last commit when pushing for big repository #10109

Merged

guillep2k reviewed Feb 4, 2020

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 4, 2020

lunny force-pushed the lunny/push_update_queue branch from 30ae45a to 558e7ac Compare February 18, 2020 09:52

lunny added this to the 1.12.0 milestone Feb 18, 2020

lunny force-pushed the lunny/push_update_queue branch from 558e7ac to bedfacd Compare February 18, 2020 12:06

lunny mentioned this pull request Feb 18, 2020

Add queue for code indexer #10332

Merged

guillep2k reviewed Feb 19, 2020

View reviewed changes

lunny force-pushed the lunny/push_update_queue branch from a9aeaac to 7492438 Compare February 19, 2020 06:41

guillep2k approved these changes Feb 20, 2020

View reviewed changes

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 Feb 20, 2020

lunny force-pushed the lunny/push_update_queue branch from 7492438 to 76317f3 Compare February 23, 2020 15:00

lunny force-pushed the lunny/push_update_queue branch from 76317f3 to 0317473 Compare April 19, 2020 02:36

lafriks modified the milestones: 1.12.0, 1.13.0 May 16, 2020

stale bot added the issue/stale label Jul 18, 2020

lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jul 18, 2020

stale bot removed the issue/stale label Jul 18, 2020

lunny force-pushed the lunny/push_update_queue branch from 0317473 to 8db5483 Compare July 26, 2020 09:11

lunny force-pushed the lunny/push_update_queue branch 3 times, most recently from 0783368 to 3f0a790 Compare September 5, 2020 11:36

lunny force-pushed the lunny/push_update_queue branch 2 times, most recently from 693b0e8 to 6dc2016 Compare September 8, 2020 00:58

lunny added 8 commits September 8, 2020 12:53

Fix test

c412b62

Add no queue for test only

c9ee7c1

improve code

de36837

Auto watch whatever branch operation

017d0d8

Fix lint

b472011

Rename noqueue to immediate

9c51284

Remove old PushUpdate function

d28cbb0

Fix tests

afc655d

lunny force-pushed the lunny/push_update_queue branch from ebf97bb to afc655d Compare September 8, 2020 04:53

zeripath reviewed Sep 8, 2020

View reviewed changes

6543 approved these changes Sep 10, 2020

View reviewed changes

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 Sep 10, 2020

zeripath and others added 2 commits September 10, 2020 16:32

Merge branch 'master' into lunny/push_update_queue

cdf235e

Merge branch 'master' into lunny/push_update_queue

a3d3165

techknowlogick and others added 3 commits September 10, 2020 21:26

Merge branch 'master' into lunny/push_update_queue

669b01d

Merge branch 'master' into lunny/push_update_queue

8861fce

Merge branch 'master' into lunny/push_update_queue

8f641ea

techknowlogick merged commit dd1a651 into go-gitea:master Sep 11, 2020

lunny deleted the lunny/push_update_queue branch September 11, 2020 23:51

zeripath mentioned this pull request Sep 13, 2020

Move git update-server-info to hooks #12826

Merged

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move all push update operations to a queue #10133

Move all push update operations to a queue #10133

lunny commented Feb 4, 2020

guillep2k Feb 4, 2020

lunny Feb 18, 2020

guillep2k Feb 19, 2020

zeripath Sep 8, 2020

lunny commented Feb 18, 2020

codecov-io commented Feb 18, 2020 •

edited

Loading

lunny commented Feb 18, 2020

guillep2k Feb 19, 2020

6543 commented Apr 7, 2020

stale bot commented Jul 18, 2020

lafriks commented Jul 25, 2020

codecov-commenter commented Sep 5, 2020 •

edited

Loading

6543 commented Sep 5, 2020 •

edited

Loading

lunny commented Sep 8, 2020

zeripath Sep 8, 2020

zeripath Sep 8, 2020

6543 Sep 10, 2020

6543 left a comment

6543 commented Sep 11, 2020

	// NewQueueService sets up the default settings for Queues
	// This is exported for tests to be able to use the queue
	func NewQueueService() {
	sec := Cfg.Section("queue")
	Queue.DataDir = sec.Key("DATADIR").MustString("queues/")
	if !filepath.IsAbs(Queue.DataDir) {
	Queue.DataDir = filepath.Join(AppDataPath, Queue.DataDir)
	}
	Queue.Length = sec.Key("LENGTH").MustInt(20)
	Queue.BatchLength = sec.Key("BATCH_LENGTH").MustInt(20)
	Queue.ConnectionString = sec.Key("CONN_STR").MustString(path.Join(AppDataPath, ""))
	Queue.Type = sec.Key("TYPE").MustString("persistable-channel")
	Queue.Network, Queue.Addresses, Queue.Password, Queue.DBIndex, _ = ParseQueueConnStr(Queue.ConnectionString)
	Queue.WrapIfNecessary = sec.Key("WRAP_IF_NECESSARY").MustBool(true)
	Queue.MaxAttempts = sec.Key("MAX_ATTEMPTS").MustInt(10)
	Queue.Timeout = sec.Key("TIMEOUT").MustDuration(GracefulHammerTime + 30*time.Second)
	Queue.Workers = sec.Key("WORKERS").MustInt(1)
	Queue.MaxWorkers = sec.Key("MAX_WORKERS").MustInt(10)
	Queue.BlockTimeout = sec.Key("BLOCK_TIMEOUT").MustDuration(1 * time.Second)
	Queue.BoostTimeout = sec.Key("BOOST_TIMEOUT").MustDuration(5 * time.Minute)
	Queue.BoostWorkers = sec.Key("BOOST_WORKERS").MustInt(5)
	Queue.QueueName = sec.Key("QUEUE_NAME").MustString("_queue")
	Queue.SetName = sec.Key("SET_NAME").MustString("")

	// Now handle the old issue_indexer configuration
	section := Cfg.Section("queue.issue_indexer")
	sectionMap := map[string]bool{}
	for _, key := range section.Keys() {
	sectionMap[key.Name()] = true
	}
	if _, ok := sectionMap["TYPE"]; !ok {
	switch Indexer.IssueQueueType {
	case LevelQueueType:
	_, _ = section.NewKey("TYPE", "level")
	case ChannelQueueType:
	_, _ = section.NewKey("TYPE", "persistable-channel")
	case RedisQueueType:
	_, _ = section.NewKey("TYPE", "redis")
	default:
	log.Fatal("Unsupported indexer queue type: %v",
	Indexer.IssueQueueType)
	}
	}
	if _, ok := sectionMap["LENGTH"]; !ok {
	_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Indexer.UpdateQueueLength))
	}
	if _, ok := sectionMap["BATCH_LENGTH"]; !ok {
	_, _ = section.NewKey("BATCH_LENGTH", fmt.Sprintf("%d", Indexer.IssueQueueBatchNumber))
	}
	if _, ok := sectionMap["DATADIR"]; !ok {
	_, _ = section.NewKey("DATADIR", Indexer.IssueQueueDir)
	}
	if _, ok := sectionMap["CONN_STR"]; !ok {
	_, _ = section.NewKey("CONN_STR", Indexer.IssueQueueConnStr)
	}

	// Handle the old mailer configuration
	section = Cfg.Section("queue.mailer")
	sectionMap = map[string]bool{}
	for _, key := range section.Keys() {
	sectionMap[key.Name()] = true
	}
	if _, ok := sectionMap["LENGTH"]; !ok {
	_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Cfg.Section("mailer").Key("SEND_BUFFER_LEN").MustInt(100)))
	}

	// Handle the old test pull requests configuration
	// Please note this will be a unique queue
	section = Cfg.Section("queue.pr_patch_checker")
	sectionMap = map[string]bool{}
	for _, key := range section.Keys() {
	sectionMap[key.Name()] = true
	}
	if _, ok := sectionMap["LENGTH"]; !ok {
	_, _ = section.NewKey("LENGTH", fmt.Sprintf("%d", Repository.PullRequestQueueLength))
	}
	}

Move all push update operations to a queue #10133

Move all push update operations to a queue #10133

Conversation

lunny commented Feb 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunny commented Feb 18, 2020

codecov-io commented Feb 18, 2020 • edited Loading

Codecov Report

lunny commented Feb 18, 2020

Choose a reason for hiding this comment

6543 commented Apr 7, 2020

stale bot commented Jul 18, 2020

lafriks commented Jul 25, 2020

codecov-commenter commented Sep 5, 2020 • edited Loading

Codecov Report

6543 commented Sep 5, 2020 • edited Loading

lunny commented Sep 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

6543 left a comment

Choose a reason for hiding this comment

6543 commented Sep 11, 2020

codecov-io commented Feb 18, 2020 •

edited

Loading

codecov-commenter commented Sep 5, 2020 •

edited

Loading

6543 commented Sep 5, 2020 •

edited

Loading