Page MenuHomeSoftware Heritage
Differential D4881

Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
ClosedPublic
Actions

Authored by douardda on Jan 19 2021, 2:49 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2444: Implement the scheduling policy for the recurrent visit scheduler
Commits
rDSCHf8627a96fed6: Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
Summary

this timestamp being actually a loading-related value, it makes more
sense to keep it in the OriginVisitStatus table.

Related to T2444.

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

douardda created this revision.Jan 19 2021, 2:49 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 19 2021, 2:49 PM
swh-public-ci added a comment.Jan 19 2021, 2:51 PM

Build is green

Patch application report for D4881 (id=17338)

Rebasing onto 5e609d5205...

Current branch diff-target is up to date.
Changes applied before test
commit f87d9d62fd087e373d1e86fdbbc6a7b5f6c09f24
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 19 14:45:09 2021 +0100

    Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
    
    this timestamp being actually a loading-related value, it makes more
    sense to keep it in the OriginVisitStatus table.
    
    Related to T2444.

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

Harbormaster completed remote builds in B18477: Diff 17338.Jan 19 2021, 2:51 PM
douardda requested review of this revision.Jan 19 2021, 2:51 PM
douardda updated this revision to Diff 17339.Jan 19 2021, 2:51 PM

forget the migration script

swh-public-ci added a comment.Jan 19 2021, 2:53 PM

Build is green

Patch application report for D4881 (id=17339)

Rebasing onto 5e609d5205...

Current branch diff-target is up to date.
Changes applied before test
commit dd905f5adeca8a4ea83f05b62b45dcc354a30d42
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 19 14:45:09 2021 +0100

    Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
    
    this timestamp being actually a loading-related value, it makes more
    sense to keep it in the OriginVisitStatus table.
    
    Related to T2444.

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

Harbormaster completed remote builds in B18478: Diff 17339.Jan 19 2021, 2:53 PM
vlorentz added a subscriber: vlorentz.Jan 19 2021, 3:13 PM
vlorentz added inline comments.
swh/scheduler/backend.py
311

why did you remove this line?

douardda added inline comments.Jan 19 2021, 3:46 PM
swh/scheduler/backend.py
311

because postgresql would not let me have it... but as I'm a noob in sql, I am very welcome to any help :-)

olasd added a subscriber: olasd.Jan 19 2021, 3:46 PM

Nice!

I've taken the liberty of reorganizing/rewriting the SQL; I have quickly tested it on a scratch database and it seems to work.

I think we'll be able to use the same three-part query scaffolding to implement other scheduling policies, only changing the filter / sort criteria, so that feels like a big win.

swh/scheduler/backend.py
305–335

I've reorganized your query in three parts:

  • selecting all the data for the next origins to visit from a join of the lister and stats table
  • updating the stats table
  • returning the selected data directly

We don't need to chain all the data through using a returning in the update_stats CTE, we can just select all the interesting data in the first select, do the bulk update, and return the data from the first CTE directly.

323

We'll probably want to be able to set this now() timestamp from the outside, for simulation purposes.

819

Maybe this would deserve a separate commit (no need for a separate diff)?

douardda added inline comments.Jan 19 2021, 3:50 PM
swh/scheduler/backend.py
305–335

awesome, thx a lot

819

I can but this is really needed here as a side effect of the new implementation of grab_next_visits() (which inserts rows in origin_visit_stats)

douardda updated this revision to Diff 17346.Jan 19 2021, 4:14 PM

Use olasd's version os the SQL query

swh-public-ci added a comment.Jan 19 2021, 4:16 PM

Build is green

Patch application report for D4881 (id=17346)

Rebasing onto 5e609d5205...

Current branch diff-target is up to date.
Changes applied before test
commit 197dcea6448b15f39fd5406ab3b741a00de8319d
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 19 14:45:09 2021 +0100

    Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
    
    this timestamp being actually a loading-related value, it makes more
    sense to keep it in the OriginVisitStatus table.
    
    Related to T2444.

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

Harbormaster completed remote builds in B18484: Diff 17346.Jan 19 2021, 4:16 PM
vsellier added a subscriber: vsellier.Jan 19 2021, 4:16 PM
vsellier added inline comments.
sql/updates/24.sql
9

Shouldn't the indexes be added here too?

douardda updated this revision to Diff 17352.Jan 19 2021, 5:49 PM

rebased

swh-public-ci added a comment.Jan 19 2021, 5:51 PM

Build is green

Patch application report for D4881 (id=17352)

Rebasing onto 0a32a31195...

Current branch diff-target is up to date.
Changes applied before test
commit f8627a96fed6fa8d4485d4092159982dd5c6362b
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jan 19 14:45:09 2021 +0100

    Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus
    
    this timestamp being actually a loading-related value, it makes more
    sense to keep it in the OriginVisitStatus table.
    
    Related to T2444.

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

Harbormaster completed remote builds in B18489: Diff 17352.Jan 19 2021, 5:52 PM
olasd accepted this revision.Jan 20 2021, 9:02 AM
This revision is now accepted and ready to land.Jan 20 2021, 9:02 AM
Closed by commit rDSCHf8627a96fed6: Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus (authored by douardda). · Explain WhyJan 20 2021, 10:05 AM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDSCHf8627a96fed6: Move the `last_scheduled` ts from ListedOrigin to OriginVisitStatus.

Revision Contents
Changeset List

PathSize
sql/
updates/
9 lines
swh/
scheduler/
43 lines
7 lines
sql/
10 lines
7 lines
tests/
110 lines
6 lines

Diff 17370

View Options

sql/updates/24.sql

Loading...
View Options

swh/scheduler/backend.py

Loading...
View Options

swh/scheduler/model.py

Loading...
View Options

swh/scheduler/sql/30-schema.sql

Loading...
View Options

swh/scheduler/sql/60-indexes.sql

Loading...
View Options

swh/scheduler/tests/test_journal_client.py

Loading...
View Options

swh/scheduler/tests/test_scheduler.py

Loading...
About · Developer information · Legal