Page MenuHomeSoftware Heritage

Add test triggering the race condition in content_add
ClosedPublic

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

Details

Summary

This minimal example triggers T2019

Test Plan

tox -e py3 -- -k TestStorageRaceConditions

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

olasd created this revision.Mon, Sep 30, 2:14 PM
olasd updated this revision to Diff 6862.Mon, Sep 30, 2:28 PM

Simplify test code

anlambert accepted this revision.Mon, Sep 30, 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.Mon, Sep 30, 2:34 PM
anlambert added inline comments.Mon, Sep 30, 2:42 PM
swh/storage/tests/test_storage.py
4050–4051

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

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

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.Mon, Sep 30, 6:19 PM

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

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

How about landing it as explicitly skipped by pytest though?

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

Mark race condition test as xfail

olasd updated this revision to Diff 6923.Tue, Oct 1, 6:07 PM

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

ardumont accepted this revision.Tue, Oct 1, 6:40 PM
This revision was automatically updated to reflect the committed changes.