Page MenuHomeSoftware Heritage

storage: Insert from temporary tables in consistent order
ClosedPublic

Authored by vlorentz on Nov 23 2022, 9:21 AM.

Details

Summary

This avoids having a transaction inserting row A then B, while another
inserts row B then A; which (probably) leads to deadlocks like this:

DeadlockDetected: deadlock detected
DETAIL:  Process 1842336 waits for ShareLock on transaction 1051957280; blocked by process 64261.
Process 64261 waits for ShareLock on transaction 1051957281; blocked by process 1842336.
HINT:  See server log for query details.
CONTEXT:  while inserting index tuple (1972253,5) in relation "origin_extrinsic_metadata"
SQL statement "insert into origin_extrinsic_metadata (id, metadata, indexer_configuration_id, from_remd_id, metadata_tsvector, mappings)

https://sentry.softwareheritage.org/share/issue/52b06caae89f4235a758887fd6817656/

This was already mitigating by sorting before inserting in temporary
tables, then expecting postgresql to read from temporary tables in the
same order rows where inserted. This is often true, but not guaranteed.

Resolves T4696.

Test Plan

No test for this, because I do not see a way to replicate this more than
existing deadlock tests do.

Diff Detail

Repository
rDCIDX Metadata indexer
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 32985
Build 51692: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 51691: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8873 (id=31995)

Rebasing onto 221d48e242...

Current branch diff-target is up to date.
Changes applied before test
commit b52d14e653811077a50f5f300d8a33bc6fe38da9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 23 09:11:21 2022 +0100

    storage: Insert from temporary tables in consistent order
    
    This avoids having a transaction inserting row A then B, while another
    inserts row B then A; which (probably) leads to deadlocks like this:
DeadlockDetected: deadlock detected
DETAIL:  Process 1842336 waits for ShareLock on transaction 1051957280; blocked by process 64261.
Process 64261 waits for ShareLock on transaction 1051957281; blocked by process 1842336.
HINT:  See server log for query details.
CONTEXT:  while inserting index tuple (1972253,5) in relation "origin_extrinsic_metadata"
SQL statement "insert into origin_extrinsic_metadata (id, metadata, indexer_configuration_id, from_remd_id, metadata_tsvector, mappings)
```

https://sentry.softwareheritage.org/share/issue/52b06caae89f4235a758887fd6817656/

This was already mitigating by sorting before inserting in temporary
tables, then expecting postgresql to read from temporary tables in the
same order rows where inserted. This is often true, but not guaranteed.

No test for this, because I do not see a way to replicate this more than
existing deadlock tests do.
See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/527/ for more details.

lgtm but this is missing the migration script update first (and the bump in the schema version somewhere?).

(i'll let you deal with the stuff that shoud disappear as you wish)

swh/indexer/sql/50-func.sql
98

This can go away (and should have already but...), i don't think we still have the content_language_* entrypoints in the indexer storage (rapid check here in storage.py confirms it).

(in another diff/commit or whatever you wish).

149

ditto for content_ctags_*...

olasd added a subscriber: olasd.

As mentioned by @ardumont, please bump the schema version and add a migration script.

(I'm a bit dubious about whether this will actually mitigate the issue, but it can't really hurt to try)

This revision is now accepted and ready to land.Nov 28 2022, 10:45 AM

bump schema version and add migration

swh/indexer/sql/50-func.sql
98

ah, I felt there was something fishy about the mismatched number of changes in .py and .sql.

I'll do it in a next diff, thanks

Build is green

Patch application report for D8873 (id=32036)

Rebasing onto 221d48e242...

Current branch diff-target is up to date.
Changes applied before test
commit f7833b7e2d98a0e19faa8c0acf0c7fc5b8b3e681
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 23 09:11:21 2022 +0100

    storage: Insert from temporary tables in consistent order
    
    This avoids having a transaction inserting row A then B, while another
    inserts row B then A; which (probably) leads to deadlocks like this:
DeadlockDetected: deadlock detected
DETAIL:  Process 1842336 waits for ShareLock on transaction 1051957280; blocked by process 64261.
Process 64261 waits for ShareLock on transaction 1051957281; blocked by process 1842336.
HINT:  See server log for query details.
CONTEXT:  while inserting index tuple (1972253,5) in relation "origin_extrinsic_metadata"
SQL statement "insert into origin_extrinsic_metadata (id, metadata, indexer_configuration_id, from_remd_id, metadata_tsvector, mappings)
```

https://sentry.softwareheritage.org/share/issue/52b06caae89f4235a758887fd6817656/

This was already mitigating by sorting before inserting in temporary
tables, then expecting postgresql to read from temporary tables in the
same order rows where inserted. This is often true, but not guaranteed.

No test for this, because I do not see a way to replicate this more than
existing deadlock tests do.
See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/528/ for more details.
swh/indexer/sql/50-func.sql
98