Add `DatabaseErrorKind` variants for isolation failures

[kestred]

Feature Request

The DatabaseErrorKind enum is intended to container error kind variants describing errors which are commonly recoverable at the application level.

I'd like to have a variant or variants representing when an isolation failure occurs in the database (e.g. an otherwise valid request fails due to a conflicting transaction occurring at the same time; such as commit serializability, dead lock, etc).

In some applications these are frequently recoverable via simple retry logic (ideally implemented with randomized retry falloff), so it should make sense to have variants for them in the enum.

Checklist

• [x] I have already looked over the issue tracker for similar issues.

[sgrif]

Seems reasonable. Can you point me at the specific error code and provide a minimal query/code to trigger the error?

[kestred]

The error codes that I'm hoping to catch are:

• 40001 | serialization_failure
• 40P01 | deadlock_detected

Typically one or the other depending on whether the transaction is constructed with build_transaction().serializable() or not.

[kestred]

This error occurs for my use case specifically when two different services were attempting to do something that conflicts, but I've not been successful building a simplified use case.

Non-Working Test (for serialization_failure)

Sourced from the postgres documentation hoping to find a minimal test case --- I'd written this, but I must've gotten something a bit wrong 😓.

#[test]
#[cfg(feature = "postgres")]
fn isolation_error_is_detected() {
    // we need custom and multiple transactions, so we can't use default transaction
    let conn = connection_without_transaction();
    conn.execute(
            "CREATE TABLE IF NOT EXISTS isolation_example (
                id SERIAL PRIMARY KEY,
                class INTEGER NOT NULL,
                value INTEGER NOT NULL
            );",
        )
        .unwrap();
    conn.execute(
            "INSERT INTO isolation_example (class, value) VALUES
                (1, 10),
                (1, 20),
                (2, 100),
                (2, 200)
             ;",
        ).unwrap();

    let other_process = ::std::thread::spawn(|| {
        let other_conn = connection_without_transaction();
        other_conn.build_transaction().serializable().run::<_, result::Error, _>(|| {
            other_conn
                .execute("SELECT SUM(value) AS sum FROM isolation_example WHERE class = 1);")
                .unwrap();

            ::std::thread::sleep(::std::time::Duration::from_millis(5));

            other_conn
                .execute("INSERT INTO isolation_example (class, value) VALUES (2, 30);")
                .unwrap();
            Ok(())
        }).ok();
    });

    let failure = conn.build_transaction().serializable().run::<_, result::Error, _>(|| {
        ::std::thread::sleep(::std::time::Duration::from_millis(2));

        conn.execute("SELECT SUM(value) FROM isolation_example WHERE class = 2;")
            .unwrap();

        other_process.join().unwrap();

        Ok(())
    });
    assert_matches!(failure, Err(DatabaseError(IsolationFailure, _)));

    // clean up because we aren't in a transaction
    conn.execute("DROP TABLE isolation_example;").unwrap();
}

I'll take another crack it at eventually...

[sgrif]

Calling the error SerializationFailure, as it does not appear to apply to any isolation levels other than SERIALIZABLE, and that is the documented text for this error in the SQLSTATE standard

[sgrif]

I've cleaned up the test case and opened #1839 for the SERIALIZABLE half of this. I'm holding off on deadlocks for now, since that's much easier to demonstrate across multiple backends (maybe not SQLite >_>), but the SQLSTATE code for it is PG specific

[Ten0]

https://github.com/diesel-rs/diesel/pull/2935/files#diff-96f0313427dbd2be25856359744270243ad151bb486851aa18bff4eeac6ba1fcR327 has gotten this particular issue worse: because the serialization failure may happen on commit attempt, in order to retry on serialization failures we've currently got to write this:

                fn is_ser_error(e: &diesel::result::Error) -> bool {
                    use diesel::result::Error;
                    match e {
                        Error::DatabaseError(diesel::result::DatabaseErrorKind::SerializationFailure, _) => true,
                        Error::CommitTransactionFailed { commit_error, .. } => is_ser_error(commit_error),
                        _ => false,
                    }
                }

This looks like a function that would have its place within Diesel, probably on the Error enum, as this will enable to add more variants to the Error enum without breaking this kind of detection for users (which is what happened to us as of #2935).

Possibly, we also don't want this to return a boolean, but instead an enum of known "isolation failures" types, so user can choose to what extent they want to retry based on the kind...

[weiznich]

@Ten0 We could probably do something to what we do here for RollbackError? That would mean we return SerializationFailure directly and everything else wrapped into a CommitTransactionFailed error?

[Ten0]

@Ten0 We could probably do something to what we do here for RollbackError? That would mean we return SerializationFailure directly and everything else wrapped into a CommitTransactionFailed error?

I'm working on a PR in that direction (backwards-incompatible) :) I'll need help on this question: https://github.com/diesel-rs/diesel/pull/3211#issuecomment-1160669506