Page MenuHomeSoftware Heritage

origin_visit_add: Fix crash when adding multiple visits to the same origin simultaneously
ClosedPublic

Authored by vlorentz on Aug 18 2022, 10:29 AM.

Details

Reviewers
olasd
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
Restricted Maniphest Task
Commits
rDSTOb5836bab0d73: origin_visit_add: Fix crash when adding multiple visits to the same origin…
Summary

This works by adding a RW lock on the row of the latest visit,
which should block other transactions until the insertion
is committed; so other transactions will generate a different
(larger) visit id

This commit also slightly rewrites how the max visit id is
computed, as we need to actually select a row to lock it,
instead of using the max() aggregate function.

Resolves T4444.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 18 2022, 10:30 AM
Harbormaster failed remote builds in B30847: Diff 29795!

add paragraph about max() in commit msg

Build is green

Patch application report for D8250 (id=29795)

Rebasing onto 5335244fc1...

Current branch diff-target is up to date.
Changes applied before test
commit f2578b91cab5830ea7582940dc5f50b40bc9711d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Aug 18 10:29:29 2022 +0200

    origin_visit_add: Fix crash when adding multiple visits to the same origin simultaneously
    
    This works by adding a RW lock on the row of the latest visit,
    which should block other transactions until the insertion
    is committed; so other transactions will generate a different
    (larger) visit id

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

Build is green

Patch application report for D8250 (id=29796)

Rebasing onto 5335244fc1...

Current branch diff-target is up to date.
Changes applied before test
commit e5543840d471e9a91e292c9dccab1157b13dac1b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Aug 18 10:29:29 2022 +0200

    origin_visit_add: Fix crash when adding multiple visits to the same origin simultaneously
    
    This works by adding a RW lock on the row of the latest visit,
    which should block other transactions until the insertion
    is committed; so other transactions will generate a different
    (larger) visit id
    
    This commit also slightly rewrites how the max visit id is
    computed, as we need to actually select a row to lock it,
    instead of using the `max()` aggregate function.

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

olasd requested changes to this revision.Aug 18 2022, 4:44 PM
olasd added a subscriber: olasd.

This is missing the database upgrade script

This revision now requires changes to proceed.Aug 18 2022, 4:44 PM

Build is green

Patch application report for D8250 (id=29883)

Rebasing onto 5335244fc1...

Current branch diff-target is up to date.
Changes applied before test
commit b5836bab0d73621458a76f886c7539bb3c9f2c32
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Aug 18 10:29:29 2022 +0200

    origin_visit_add: Fix crash when adding multiple visits to the same origin simultaneously
    
    This works by adding a RW lock on the row of the latest visit,
    which should block other transactions until the insertion
    is committed; so other transactions will generate a different
    (larger) visit id
    
    This commit also slightly rewrites how the max visit id is
    computed, as we need to actually select a row to lock it,
    instead of using the `max()` aggregate function.

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

ardumont added a subscriber: ardumont.

The blocking point has been lifted.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 25 2022, 10:01 AM
This revision was automatically updated to reflect the committed changes.