Page MenuHomeSoftware Heritage
Differential D2041

Add test triggering the race condition in content_add
ClosedPublic
Actions

Authored by olasd on Sep 30 2019, 2:14 PM.

Details

Reviewers
anlambert
ardumont
Group Reviewers
Reviewers
Commits
rDSTOCc1ce3a14b91b: Add test triggering the race condition in content_add
rDSTOc1ce3a14b91b: Add test triggering the race condition in content_add
Summary

This minimal example triggers T2019

Test Plan

tox -e py3 -- -k TestStorageRaceConditions

Diff Detail

Repository
rDSTO Storage manager
Branch
allow-racecondition
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8067
Build 11629: tox-on-jenkinsJenkins
Build 11628: arc lint + arc unit

Event Timeline

olasd created this revision.Sep 30 2019, 2:14 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 30 2019, 2:14 PM
swh-public-ci added a comment.Sep 30 2019, 2:15 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/655/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/655/console

Harbormaster failed remote builds in B8003: Diff 6861!Sep 30 2019, 2:15 PM
olasd updated this revision to Diff 6862.Sep 30 2019, 2:28 PM

Simplify test code

swh-public-ci added a comment.Sep 30 2019, 2:32 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/656/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/656/console

Harbormaster failed remote builds in B8004: Diff 6862!Sep 30 2019, 2:32 PM
anlambert accepted this revision.Sep 30 2019, 2:34 PM
anlambert added a subscriber: anlambert.

Looks good to me ! I was also trying to write such a test but you were faster.

This revision is now accepted and ready to land.Sep 30 2019, 2:34 PM
anlambert added inline comments.Sep 30 2019, 2:42 PM
swh/storage/tests/test_storage.py
4049–4050

For your information, the test also fails when not providing the db and cur parameter. Are they really needed ?

olasd added inline comments.Sep 30 2019, 3:48 PM
swh/storage/tests/test_storage.py
4049–4050

They are not really (in the current state of this code).

Technically, to be sure that the race condition triggers all the time, we need to add a synchronization point between the threads, so that we are sure that both transactions are open at the same time. This means synchronizing before the content_add call, but after db.transaction().

ardumont accepted this revision.Sep 30 2019, 6:19 PM
olasd added a comment.Sep 30 2019, 10:37 PM

(I'm not landing a purposefully failing test, fwiw)

zack added a subscriber: zack.Sep 30 2019, 10:41 PM

How about landing it as explicitly skipped by pytest though?

olasd updated this revision to Diff 6917.Oct 1 2019, 5:47 PM

Mark race condition test as xfail

swh-public-ci added a comment.Oct 1 2019, 5:53 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/658/ for more details.

Harbormaster completed remote builds in B8061: Diff 6917.Oct 1 2019, 5:53 PM
olasd updated this revision to Diff 6923.Oct 1 2019, 6:07 PM

♪┏(・o･)┛♪┗ ( ･o･) ┓♪ do the rebase dance party

swh-public-ci added a comment.Oct 1 2019, 6:12 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/659/ for more details.

Harbormaster completed remote builds in B8067: Diff 6923.Oct 1 2019, 6:12 PM
ardumont accepted this revision.Oct 1 2019, 6:40 PM
Closed by commit rDSTOc1ce3a14b91b: Add test triggering the race condition in content_add (authored by olasd). · Explain WhyOct 2 2019, 1:30 PM
This revision was automatically updated to reflect the committed changes.
olasd added a commit: rDSTOc1ce3a14b91b: Add test triggering the race condition in content_add.
olasd added a commit: rDSTOCc1ce3a14b91b: Add test triggering the race condition in content_add.Oct 30 2019, 5:22 PM

Revision Contents
Changeset List

PathSize
swh/
storage/
tests/
44 lines

Diff 6923

View Options

swh/storage/tests/test_storage.py

Loading...
About · Developer information · Legal