Skip to content

Fix SQLAlchemy commit errors in Index.bulk_delete

Richard Kennedy requested to merge richken/fix_cleanup_sql_commit_errors into master

Description

This MR addresses two problems, index.bulk_delete raising StaleDataErrors and index.bulk_delete creating deadlocks.

StaleDataErrors

This is the simpler of the two problems. When index.bulk_delete is run concurrently by different processes, the expected number of rows may not be deleted. This is because another process may have deleted/locked them before us. The best resolution to this problem is to probably ignore/suppress the error. The rows will be deleted anyway by the other process anyway. Even if the other process fails to delete these rows, bgd-cleanup will assign these rows to be deleted eventually. This MR makes sure that we do not rollback, log, or raise errors on this exception. Since it happens common enough, that doing any of the aforementioned would be very inefficient.

DeadLocks

index.bulk_delete generates deadlocks when concurrent processes attempt to delete multiple rows at once. For example, assume two bgd-cleanup instances are attempting to delete the same two digests. A deadlock can happen if process 1 acquires a lock on row 1, and blocks on access to row 2. While, process 2 acquires a lock on row 2, and blocks on access to row 1.

1 Skip Locked

There are a few ways to deal with this problem. The solution this MR uses, is to use the Postgres skip_locked feature to skip rows which are locked (not wait on them). This should avoid deadlocks. The downside of this, is that there are inefficiencies associated with this. Moreover, not all databases support skip_locked (Postgres & Oracle do but MySQL doesn't). We could make the solution more robust by using the strategy explained here: https://www.2ndquadrant.com/en/blog/what-is-select-skip-locked-for-in-postgresql-9-5/. Using this method will also resolve the StaleDataErrors errors as well. Unfortunately, when testing this method, it was found to be far too slow for our use case.

2 Sharding

Alternatively, we could shard our data so that different instances of bgd_cleanup do not attempt to delete the same rows. One approach to doing this could be to associate a cleanup instances with a unique name and then associate its marked digests with that name. However, this creates problems if a particular bgd cleanup instance crashes.

Another approach could be to only have bgd_cleanup delete the digests which it marked for deletion. However, this might push the deadlock problem up the mark_n_bytes_as_deleted method.

Results

While running 15 cleanup instances, with a batch size of 5 MB, and average file size of 5kb, and a high watermark of 100mb. No deadlocks were generated. Master for reference would sometimes fail to cleanup any items due to constantly running into deadlocks.

Edited by Richard Kennedy

Merge request reports