Grinnz / Mojo-SQLite

Mojo::SQLite - A tiny Mojolicious wrapper for SQLite
https://metacpan.org/pod/Mojo::SQLite
Other
27 stars 12 forks source link

Fix race condition about "UNIQUE constraint failed" #24

Closed okurz closed 2 years ago

okurz commented 2 years ago

We observed a problem where two services are accessing the same SQLite database and one of the processes would run into an error "DBD::SQLite::st execute failed: UNIQUE constraint failed: mojo_migrations.name". The problem is that the method "_active" first reads outside the database transaction lock object and then writes if no migration table version is found so it is two queries which are not protected from concurrent processes.

This commit removes the "optimization" attempt of reading (and writing) outside the database transaction and only doing a single but protected attempt. This has a potential minor performance implication but considering that this only affects actual migrations we consider this acceptable.

Verified within the scope of https://progress.opensuse.org/issues/108125 using the so called worker cacherservice for "openQA" with the following command for manual verification:

for i in {1..10000}; do echo "## Run $i" && \
systemctl stop openqa-worker-cacheservice-minion.service openqa-worker-cacheservice.service && \
rm -f /var/lib/openqa/cache/cache.sqlite* && \
systemctl start openqa-worker-cacheservice-minion.service openqa-worker-cacheservice.service && \
for j in {1..30}; do echo "waiting for sqlite" && \
test -f /var/lib/openqa/cache/cache.sqlite && break || sleep 1; done && \
journalctl --since=today _SYSTEMD_UNIT=openqa-worker-cacheservice.service + _SYSTEMD_UNIT=openqa-worker-cacheservice-minion.service | grep -c 'UNIQUE';
done

Reference: https://progress.opensuse.org/issues/108125

okurz commented 2 years ago

https://github.com/Grinnz/Mojo-SQLite/pull/25 is the alternative

Grinnz commented 2 years ago

Merged #25 instead, thank you