Page MenuHomeSoftware Heritage
Differential D5520

Route priority tasks to dedicated save code now queues
ClosedPublic
Actions

Authored by ardumont on Apr 14 2021, 12:51 PM.

Details

Reviewers
olasd
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3084: Fast track save code now requests
Commits
rDSCH17052c4cfa39: Route priority tasks to dedicated save code now queues
Summary

This splits the calls to read tasks into 2 calls, one for tasks with no
priority (standard), another call for tasks with priority. If any tasks with priority
are detected, they are routed to dedicated save_code_now: prefixed named queues (per
task type).

Related to T3084

Test Plan

tox and docker (D5526) and partial successful deployment of this on staging

Diff Detail

Repository
rDSCH Scheduling utilities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Apr 14 2021, 12:51 PM
swh-public-ci added a comment.Apr 14 2021, 12:53 PM

Build is green

Patch application report for D5520 (id=19686)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit 3d3ec7ecddbcf4bd64e4eee7523b3b6ff198fe9a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    wip: Reroute priority tasks to dedicated save code now queues
    
    Tests within dockers looks like they are doing the job.
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/308/ for more details.

Harbormaster completed remote builds in B20715: Diff 19686.Apr 14 2021, 12:53 PM
ardumont requested review of this revision.Apr 14 2021, 12:53 PM
ardumont edited the summary of this revision. (Show Details)Apr 14 2021, 12:55 PM
ardumont edited the test plan for this revision. (Show Details)
ardumont added inline comments.
swh/scheduler/celery_backend/runner.py
140–141

I tried to pass along other parameters instead of touching the name parameter to no avail:

  • queue_name (not shown in the signature of send_task so it's a noop
  • route_name but that made somehow other celery worker (lister, indexer, ...) try and consume it somehow...

The only thing i made comply was the content of this diff, touching our route_for_tasks routing setup.

ardumont edited the summary of this revision. (Show Details)Apr 14 2021, 1:06 PM
ardumont updated this revision to Diff 19687.Apr 14 2021, 1:15 PM

Rework commit message

  • Drop logger.error instructions
  • Add route_for_task tests
ardumont retitled this revision from wip: Reroute priority tasks to dedicated save code now queues to Reroute priority tasks to dedicated save code now queues.Apr 14 2021, 1:15 PM
ardumont edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Apr 14 2021, 1:17 PM

Build is green

Patch application report for D5520 (id=19687)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit 19f38785d74fc19e53b85750d783928545ae017f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Reroute priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks with no priority and tasks with priority in 2 calls.
    If tasks with priority are detected, they are routed to dedicated `save_code_now:`
    prefixed named queues.
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/309/ for more details.

Harbormaster completed remote builds in B20716: Diff 19687.Apr 14 2021, 1:17 PM
anlambert accepted this revision.Apr 14 2021, 2:03 PM
anlambert added a subscriber: anlambert.

Looks good to me.

swh/scheduler/celery_backend/runner.py
27

s/exists/exist/

This revision is now accepted and ready to land.Apr 14 2021, 2:03 PM
ardumont updated this revision to Diff 19693.Apr 14 2021, 2:23 PM

Adapt according to review: fix typo (thx)

Rework docstring sentence and commit message

ardumont edited the summary of this revision. (Show Details)Apr 14 2021, 2:24 PM
swh-public-ci added a comment.Apr 14 2021, 2:26 PM

Build is green

Patch application report for D5520 (id=19693)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit 84d554068d395359ffee0dcbc80ced2a91970c0d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Reroute priority tasks to dedicated save code now queues
    
    This splits the calls into 2 reads, one for tasks with no priority, another for tasks
    with priority. If tasks with priority are detected, they are routed to dedicated
    `save_code_now:` prefixed named queues (per task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/310/ for more details.

Harbormaster completed remote builds in B20722: Diff 19693.Apr 14 2021, 2:26 PM
ardumont added inline comments.Apr 14 2021, 2:37 PM
swh/scheduler/celery_backend/runner.py
96

It might be best to give some number of tasks to retrieve here.

ardumont updated this revision to Diff 19695.Apr 14 2021, 3:36 PM
  • Explicitely take no more priority tasks from the main grab instructions
  • Limit the number of priority tasks to retrieve
  • Add missing log and statsd instructions for those priority tasks
swh-public-ci added a comment.Apr 14 2021, 3:38 PM

Build is green

Patch application report for D5520 (id=19695)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit b95219698477498e6495f363a423412b92fd23a4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Reroute priority tasks to dedicated save code now queues
    
    This splits the calls into 2 reads, one for tasks with no priority, another for tasks
    with priority. If tasks with priority are detected, they are routed to dedicated
    `save_code_now:` prefixed named queues (per task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/311/ for more details.

Harbormaster completed remote builds in B20724: Diff 19695.Apr 14 2021, 3:38 PM
ardumont added a comment.EditedApr 14 2021, 4:24 PM

It seems my succesfull runs was somehow wrong as I don't seem to be able to
reproduce it [1].

Something is amiss in my understanding somewhere. I need to dig in further.

[1]

...
swh-loader_1                    |                 .> celery           exchange=celery(direct) key=celery
swh-loader_1                    |                 .> save_code_now:swh.loader.git.tasks.UpdateGitRepository exchange=save_code_now:swh.loader.git.tasks.UpdateGitRepository(direct) key=save_code_now:swh.loader.git.tasks.UpdateGitRepository
...
swh-loader_1                    | [2021-04-14 13:25:28,019: ERROR/MainProcess] Received unregistered task of type 'save_code_now:swh.loader.git.tasks.UpdateGitRepository'.
swh-loader_1                    | The message has been ignored and discarded.
swh-loader_1                    |
swh-loader_1                    | Did you remember to import the module containing this task?
swh-loader_1                    | Or maybe you're using relative imports?
swh-loader_1                    |
swh-loader_1                    | Please see
swh-loader_1                    | http://docs.celeryq.org/en/latest/internals/protocol.html
swh-loader_1                    | for more information.
swh-loader_1                    |
swh-loader_1                    | The full contents of the message body was:
swh-loader_1                    | b'\x93\x90\x81\xa3url\xd90https://github.com/opensearch-project/OpenSearch\x84\xa9callbacks\xc0\xa8errbacks\xc0\xa5chain\xc0\xa5chord\xc0' (93b)
swh-loader_1                    | Traceback (most recent call last):
swh-loader_1                    |   File "/srv/softwareheritage/venv/lib/python3.7/site-packages/celery/worker/consumer/consumer.py", line 555, in on_task_received
swh-loader_1                    |     strategy = strategies[type_]
swh-loader_1                    | KeyError: 'save_code_now:swh.loader.git.tasks.UpdateGitRepository'
ardumont planned changes to this revision.Apr 14 2021, 4:24 PM
ardumont added inline comments.Apr 14 2021, 5:03 PM
swh/scheduler/celery_backend/runner.py
140–141

ah! I found back the queue parameter which is not documented directly in the send_task but in the redirected doc in the task_async definition...
And it works, locally... which i'll triple check to avoid the surprise already had once...

ardumont updated this revision to Diff 19702.Apr 14 2021, 5:14 PM

Fix so it actually works reproductibly!

This revision is now accepted and ready to land.Apr 14 2021, 5:14 PM
swh-public-ci added a comment.Apr 14 2021, 5:17 PM

Build has FAILED

Patch application report for D5520 (id=19702)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit ca98fe47f2a53b04f1f3307be336c24b65a37276
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Reroute priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/312/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/312/console

Harbormaster failed remote builds in B20730: Diff 19702!Apr 14 2021, 5:17 PM
ardumont mentioned this in D5526: conf/loader: Declare save code now queues to consume from.Apr 14 2021, 5:19 PM
ardumont edited the test plan for this revision. (Show Details)Apr 14 2021, 5:19 PM

Build has FAILED

yeah, all this to see if you bot would follow me anywhere, apparently not (good ;)

ardumont updated this revision to Diff 19705.Apr 14 2021, 5:21 PM

Fix test config typo

ardumont updated this revision to Diff 19706.Apr 14 2021, 5:22 PM

Rework commit message

ardumont retitled this revision from Reroute priority tasks to dedicated save code now queues to Route priority tasks to dedicated save code now queues.Apr 14 2021, 5:22 PM
ardumont edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Apr 14 2021, 5:24 PM

Build is green

Patch application report for D5520 (id=19705)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit 4e3764d3874d9259b4e8b889253b1d74c8036fcb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Reroute priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/313/ for more details.

Harbormaster completed remote builds in B20732: Diff 19705.Apr 14 2021, 5:24 PM
swh-public-ci added a comment.Apr 14 2021, 5:26 PM

Build is green

Patch application report for D5520 (id=19706)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit fb6f4a442bf25f8d32c12e5ebb4757195f7d95aa
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/314/ for more details.

Harbormaster completed remote builds in B20733: Diff 19706.Apr 14 2021, 5:26 PM
ardumont added inline comments.Apr 14 2021, 5:35 PM
swh/scheduler/celery_backend/runner.py
96

yes, and i took the max_queue_length value configured from the task type.
There is no size check for the current value of the corresponding queue.

As a first implementation, we agreed it'd be enough that way and we'll see
what actually happens when deployed.

The thought process is that the quantity should be relatively marginal.

anlambert added inline comments.Apr 14 2021, 5:55 PM
swh/scheduler/celery_backend/runner.py
141–153

You should use a dict to fill with parameters and call app.send_task after the if/else block instead of calling it in each execution branch.

ardumont added inline comments.Apr 14 2021, 6:05 PM
swh/scheduler/celery_backend/runner.py
141–153

totally

ardumont mentioned this in T3084: Fast track save code now requests.Apr 14 2021, 6:38 PM
ardumont updated this revision to Diff 19719.Apr 14 2021, 6:56 PM

Refactor to avoid repetition in app.send_task calls

ardumont marked an inline comment as done.Apr 14 2021, 6:58 PM
swh-public-ci added a comment.Apr 14 2021, 6:58 PM

Build is green

Patch application report for D5520 (id=19719)

Rebasing onto 3e2ae3d46d...

Current branch diff-target is up to date.
Changes applied before test
commit a08ba1440e945be782e55d7f5d672eb6704cb6c6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/315/ for more details.

Harbormaster completed remote builds in B20741: Diff 19719.Apr 14 2021, 6:58 PM
ardumont added a comment.Apr 15 2021, 11:08 AM

I'm trying to make the tests comply to cover the missing part about priority tasks.
And i found some tests already running the runner part.

ardumont added a comment.Apr 15 2021, 11:57 AM

I'm trying to make the tests comply to cover the missing part about priority tasks.
And i found some tests already running the runner part.

Almost there, tasks are scheduled correctly and send to the rightful queue but... I've
yet to understand how to configure the test worker to make it consume that new message...

ardumont updated this revision to Diff 19753.Apr 15 2021, 12:06 PM

Add test on scheduling task with priority

Note that the consumption from the new priority queue is not there yet though. I'm
struggling a bit with our celery configuration for that part (make the damn worker
subscribe to that new queue)

swh-public-ci added a comment.Apr 15 2021, 12:09 PM

Build is green

Patch application report for D5520 (id=19753)

Rebasing onto bfc1a87bff...

First, rewinding head to replay your work on top of it...
Applying: Route priority tasks to dedicated save code now queues
Changes applied before test
commit ce3dfc66b7eebfc1c5e647672df8f62fce3eb2e1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/317/ for more details.

Harbormaster completed remote builds in B20771: Diff 19753.Apr 15 2021, 12:09 PM
ardumont updated this revision to Diff 19756.Apr 15 2021, 1:24 PM

Rebase

ardumont mentioned this in D5535: tests: Complete checks on message with priority consumption.Apr 15 2021, 1:26 PM
swh-public-ci added a comment.Apr 15 2021, 1:27 PM

Build is green

Patch application report for D5520 (id=19756)

Rebasing onto bfc1a87bff...

Current branch diff-target is up to date.
Changes applied before test
commit 17052c4cfa39f7e53e4f7392586b9cd3bf84cba3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 14 12:49:47 2021 +0200

    Route priority tasks to dedicated save code now queues
    
    This splits the calls to read tasks into 2 calls, one for tasks with no
    priority (standard), another call for tasks with priority. If any tasks with priority
    are detected, they are routed to dedicated `save_code_now:` prefixed named queues (per
    task type).
    
    Related to T3084

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/318/ for more details.

Harbormaster completed remote builds in B20774: Diff 19756.Apr 15 2021, 1:27 PM
ardumont edited the summary of this revision. (Show Details)Apr 15 2021, 1:27 PM
ardumont edited the test plan for this revision. (Show Details)
Closed by commit rDSCH17052c4cfa39: Route priority tasks to dedicated save code now queues (authored by ardumont). · Explain WhyApr 15 2021, 1:31 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSCH17052c4cfa39: Route priority tasks to dedicated save code now queues.

Revision Contents
Changeset List

PathSize
swh/
scheduler/
celery_backend/
67 lines
tests/
96 lines
18 lines

Diff 19759

View Options

swh/scheduler/celery_backend/runner.py

Loading...
View Options

swh/scheduler/tests/test_celery_tasks.py

Loading...
View Options

swh/scheduler/tests/test_config.py

Loading...
About · Developer information · Legal