Page MenuHomeSoftware Heritage
Differential D7647

Update the the listing and loading scheduling architecture doc
ClosedPublic
Actions

Authored by douardda on Apr 25 2022, 4:01 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDDOCb6c59c460142: Update the the listing and loading scheduling architecture doc
Summary

There are titles on most of the svg objects, unfortunately these are not visible in sphinx...

Diff Detail

Event Timeline

douardda created this revision.Apr 25 2022, 4:01 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 25 2022, 4:01 PM
swh-public-ci added a comment.Apr 25 2022, 4:10 PM

Build has FAILED

Patch application report for D7647 (id=27670)

Rebasing onto b90babd909...

First, rewinding head to replay your work on top of it...
Applying: Update the the listing and loading scheduling architecture doc
Changes applied before test
commit 86bd52b70a490bfe867f352170a4a2b57982b072
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 25 2022, 4:10 PM
Harbormaster failed remote builds in B28727: Diff 27670!
douardda updated this revision to Diff 27679.Apr 25 2022, 5:43 PM

rebase

swh-public-ci added a comment.Apr 25 2022, 5:53 PM

Build has FAILED

Patch application report for D7647 (id=27679)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit bc13d85a4bac5c02987a53244fc4821b95d7d5fa
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 25 2022, 5:53 PM
Harbormaster failed remote builds in B28738: Diff 27679!
swh-public-ci added a comment.Apr 26 2022, 12:07 PM

Build has FAILED

Patch application report for D7647 (id=27679)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit bc13d85a4bac5c02987a53244fc4821b95d7d5fa
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

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

douardda updated this revision to Diff 27712.Apr 26 2022, 12:19 PM

fix a sphinx warning

swh-public-ci added a comment.Apr 26 2022, 12:36 PM

Build is green

Patch application report for D7647 (id=27712)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit bcca817ce5b8b6de45756254597942f614b2aa05
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

See https://jenkins.softwareheritage.org/job/DDOC/job/build-on-diff/161/ for more details.

Harbormaster completed remote builds in B28769: Diff 27712.Apr 26 2022, 12:36 PM
douardda requested review of this revision.Apr 26 2022, 12:36 PM
vlorentz added a subscriber: vlorentz.Apr 26 2022, 12:52 PM

could you show the two images in the diff's description?

docs/architecture/overview.rst
72–81

You are using the syntax for the definition list, but in these two items you start a sentence in the title and finish it in the definition.

You should probably add a line break before "tasks" in both titles.

olasd added a subscriber: olasd.Apr 26 2022, 1:02 PM

I have a few nits, but overall the text looks good to me, thank you!

Would you mind adding a rendered version of the plantuml diagram to the diff description?

docs/architecture/overview.rst
52–53

I think that sentence is backwards: the scheduler only pulls SWH visit information from the journal, update information is pulled by the lister.

119
148
douardda added a comment.Apr 26 2022, 1:51 PM

Fichier archi scheduler:

UML 1: listing

UML 2: loading

General rendering:


vlorentz added a comment.Apr 26 2022, 1:56 PM

Thanks. On the diagram, it looks like 9' is causing 12 (via 10 and 11), which is not true (yet! D7332 isn't agreed upon) and could be very confusing.

This is because 9' writes to the origin_visit table, but 10/11/12 are based on the origin table/topic.

Could you split swh-storage in two, to avoid this misunderstanding?

The rest LGTM

douardda updated this revision to Diff 27776.Apr 26 2022, 2:05 PM
douardda marked 2 inline comments as done.

typos and better rst syntax as suggested by vlorentz

Note: the rephrasing needed in the Journal section is stil to be done.

olasd added a comment.Apr 26 2022, 2:20 PM
In D7647#200065, @vlorentz wrote:

Thanks. On the diagram, it looks like 9' is causing 12 (via 10 and 11), which is not true (yet! D7332 isn't agreed upon) and could be very confusing.

This is because 9' writes to the origin_visit table, but 10/11/12 are based on the origin table/topic.

Could you split swh-storage in two, to avoid this misunderstanding?

To clear the misunderstanding maybe the label for 10 could be Consume final "origin visit status" entries?

You could split the "visit scheduler" part of the scheduler api into two lines (listed origins + visit stats) but that's going into a lot of detail.

Maybe the celery box could use another column for the save code now tasks (and their dedicated celery workers) that still go through the task-based scheduler.

swh-public-ci added a comment.Apr 26 2022, 2:23 PM

Build is green

Patch application report for D7647 (id=27776)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit d5138338570d7bf4bd8ed92d80fa2384a4cd6744
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

See https://jenkins.softwareheritage.org/job/DDOC/job/build-on-diff/162/ for more details.

Harbormaster completed remote builds in B28835: Diff 27776.Apr 26 2022, 2:23 PM
douardda added a comment.Apr 26 2022, 5:10 PM
In D7647#200113, @olasd wrote:
In D7647#200065, @vlorentz wrote:

Thanks. On the diagram, it looks like 9' is causing 12 (via 10 and 11), which is not true (yet! D7332 isn't agreed upon) and could be very confusing.

This is because 9' writes to the origin_visit table, but 10/11/12 are based on the origin table/topic.

Could you split swh-storage in two, to avoid this misunderstanding?

To clear the misunderstanding maybe the label for 10 could be Consume final "origin visit status" entries?

IMHO it's a detail; generally speaking, we do update visits info/stats from this origin-visit-status topic; the fact we are only using some of these messages is a minor detail.

You could split the "visit scheduler" part of the scheduler api into two lines (listed origins + visit stats) but that's going into a lot of detail.

Maybe the celery box could use another column for the save code now tasks (and their dedicated celery workers) that still go through the task-based scheduler.

I find this diagram already too complex. I'd prefer not to add more details.

I agree it could be useful to add a section for "save code now" (and save forge now)

docs/architecture/overview.rst
72–81

I did that but I wasn't convinced by the result. but I'm not convinced by this version either...

Current:

Proposed:

Both are fine for me, so I'll go for 2.

douardda updated this revision to Diff 27786.Apr 26 2022, 5:16 PM

rephrase (journal usage by the scheduler)

swh-public-ci added a comment.Apr 26 2022, 5:33 PM

Build has FAILED

Patch application report for D7647 (id=27786)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit b6c59c46014215196e79d9b4377b9061e323dcfd
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

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

Harbormaster failed remote builds in B28847: Diff 27786!Apr 26 2022, 5:33 PM
swh-public-ci added a comment.Apr 26 2022, 6:08 PM

Build has FAILED

Patch application report for D7647 (id=27786)

Rebasing onto b90babd909...

Current branch diff-target is up to date.
Changes applied before test
commit b6c59c46014215196e79d9b4377b9061e323dcfd
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Apr 25 15:57:34 2022 +0200

    Update the the listing and loading scheduling architecture doc

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

olasd accepted this revision.Apr 26 2022, 6:12 PM

I think the separate mechanic for save code now is much more than a detail; it's the main reason we've kept the old task scheduler.

But this diff is definitely a big step up from the status quo, and good enough to be published as is; we can improve on it later!

This revision is now accepted and ready to land.Apr 26 2022, 6:12 PM
This revision was landed with ongoing or failed builds.Apr 26 2022, 6:14 PM
Closed by commit rDDOCb6c59c460142: Update the the listing and loading scheduling architecture doc (authored by douardda). · Explain Why
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDDOCb6c59c460142: Update the the listing and loading scheduling architecture doc.

Revision Contents
Changeset List

PathSize
docs/
architecture/
169 lines
images/
3340 lines
77 lines
6 lines

Diff 27712

View Options

docs/architecture/overview.rst

Loading...
View Options

docs/images/lister-loader-scheduling-architecture.svg

Loading...
View Options

docs/images/tasks-git-loader.uml

Loading...
View Options

docs/images/tasks-lister.uml

Loading...
About · Developer information · Legal