Auto merge of #3414 - pietroalbini:deadlock, r=jtgeibel

Prevent transaction deadlocks in DownloadsCounter

While analyzing the behavior of `DownloadsCounter` (introduced in #3413) in production, we noticed that there were a few cases of transaction deadlocks tonight in the background thread persisting the download counts.

I did some investigation on the cause of this and came up with this fix. As [recommended by the PostgreSQL docs](https://www.postgresql.org/docs/11/explicit-locking.html#LOCKING-DEADLOCKS), to avoid deadlocks when multiple transactions affect multiple rows in a table, the affected keys needs to be modified in a consistent order across the transactions. We implement that by sorting by `version_id` the items we're going to insert.

<details><summary>Local reproduction instructions</summary>

The exact error we saw in production can be reproduced by opening two instances of `psql` locally and running those two set of queries in each `psql` (without running `COMMIT`):

```sql
-- First instance of psql
BEGIN;
INSERT INTO version_downloads (version_id) VALUES (1) ON CONFLICT (version_id, date) DO UPDATE SET downloads = excluded.downloads + 1;

-- Second instance of psql
BEGIN;
INSERT INTO version_downloads (version_id) VALUES (2) ON CONFLICT (version_id, date) DO UPDATE SET downloads = excluded.downloads + 1;
```

Then, once the commands are sent to both instances of `psql`, sending these other commands will cause the deadlock:

```sql
-- First instance of psql
INSERT INTO version_downloads (version_id) VALUES (2) ON CONFLICT (version_id, date) DO UPDATE SET downloads = excluded.downloads + 1;

-- Second instance of psql
INSERT INTO version_downloads (version_id) VALUES (1) ON CONFLICT (version_id, date) DO UPDATE SET downloads = excluded.downloads + 1;
```

Instead, if both transactions try to insert first `1` and then `2` the first `psql` will work fine, and the second `psql` will wait for the first `psql` to `COMMIT` before applying the changes. That's the behavior we want and it's the one this PR implements.

</details>

r? `@jtgeibel`

Author: bors

src/downloads_counter.rs

@@ -126,10 +126,31 @@ impl DownloadsCounter {
             counted_downloads += count;
             counted_versions += 1;
 
-            to_insert.push((version_id.eq(*key), downloads.eq(count as i32)));
+            to_insert.push((*key, count));
         }
 
         if !to_insert.is_empty() {
+            // The rows we're about to insert need to be sorted to avoid deadlocks when multiple
+            // instances of crates.io are running at the same time.
+            //
+            // In PostgreSQL a transaction modifying a row locks that row until the transaction is
+            // committed. Multiple transactions inserting rows into a table could end up
+            // deadlocking each other though: PostgreSQL will detect that deadlock, abort one of
+            // the transactions and allow the other one to continue. We don't want that to happen,
+            // as we'd lose the downloads from the aborted transaction.
+            //
+            // Ensuring the rows are inserted in a consistent order (in our case by sorting them by
+            // the version ID) will prevent deadlocks from occuring. For more information:
+            //
+            //     https://www.postgresql.org/docs/11/explicit-locking.html#LOCKING-DEADLOCKS
+            //
+            to_insert.sort_by_key(|(key, _)| *key);
+
+            let to_insert = to_insert
+                .into_iter()
+                .map(|(key, count)| (version_id.eq(key), downloads.eq(count as i32)))
+                .collect::<Vec<_>>();
+
             diesel::insert_into(version_downloads)
                 .values(&to_insert)
                 .on_conflict((version_id, date))