Akonadiserver: rework handling of database deadlocks.
ClosedPublic

Authored by dfaure on Apr 6 2019, 9:47 PM.

Details

Summary

We can't just replay the SQL commands, that's too low-level: if we do
INSERT, then use the resulting ID into e.g. another INSERT command, at replay
time we might get a different ID from the first command, and then insert
the wrong ID in the second one...

So instead we want to re-run the entire handler's parseStream(), which we
do by throwing a DbDeadlockException (from QueryBuilder), and catching that
in the caller (Connection) using a helper class (DbDeadlockCatcher, unittested)
which retries up to 5 times.

For this to work, it means:

  • we must also rollback any chances made to in-memory caches (done by

clearing those caches, good enough since DB deadlocks are rare)

  • ItemModifyJob which sends parts on demand, shouldn't delete after

sending (it might be asked again) -- this was hit by GidTest (fetchAndSetGid).

Still TODO:

  • delaying the updating of intervalChecker and cacheCleaner by NotificationCollector until commit time;
  • checking other users of QueryBuilder (who now can get a DbDeadlockException...)
Test Plan

all this was triggered by a unittest that creates 10 sessions
and adds 10 different attributes to the same time, one in each session. This very
often hits the DB deadlock scenario.

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure created this revision.Apr 6 2019, 9:47 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 6 2019, 9:47 PM
dfaure requested review of this revision.Apr 6 2019, 9:47 PM
dvratil requested changes to this revision.Apr 6 2019, 10:56 PM
dvratil added inline comments.
src/server/connection.cpp
50

Use "" instead of <>, it is a local include (while private is another library)

src/server/connection.h
131

const&

src/server/storage/datastore.cpp
69

Not related, keep it in DataStore, please.

src/server/storage/dbdeadlockcatcher.h
48

Use a named constant instead of a magic number.

src/server/storage/transaction.cpp
20 ↗(On Diff #55606)

Not needed.

This revision now requires changes to proceed.Apr 6 2019, 10:56 PM
dfaure updated this revision to Diff 55629.Apr 7 2019, 7:50 AM

Make requested changes

dvratil accepted this revision.Apr 7 2019, 9:25 AM
This revision is now accepted and ready to land.Apr 7 2019, 9:25 AM
This revision was automatically updated to reflect the committed changes.