Custom savepoint names for subtransactions

[nvb]

Of the few Postgres drivers that support subtransactions, it seems like most permit access to the subtransaction's "name". For instance, the official C++ driver takes a name in as a parameter when creating a subtractions. This name is then used as the subtransaction's savepoint name where applicable. It seems like this driver currently hardcodes the name "sp" in the three places that a name is needed.

Would you be open to providing access to this savepoint name on subtransactions through either:

• an added parameter to the current constructor (breaking change)
• a secondary constructor with the extra parameter (eg. transactionWithName or namedTransaction)
• a setter method (which would require extra logic to prevent it's use on non-nested transactions)

If so, I'd be happy to send a PR with the change!

[sfackler]

Out of curiosity, what's the use case for doing this?

[nvb]

Another option would be to add the savepoint name into the transaction::Config struct. This approach seems pretty well precedented, as it's where transaction options like isolation level are exposed.

The use case for savepoints in relation to CockroachDB is an artifact of our optimistic concurrency control model. Because our system relies on OCC, we are able to able to perform distributed atomic transactions system-wide without locking (describer here if you're curious). However, a result of this is that transaction restarts (performed with a priority ratcheting mechanism) are more common than in many other databases. As such, we need to expose retryable transactions to drivers, and our current solution for this is a nested transaction with a sentinel savepoint name. This allows nested transaction rollbacks on retryable errors to remain in the same parent transaction, and benefit from the increased priority on their retries'. There are a few simple examples here.

[sfackler]

Interesting! So transactions and savepoints not named cockroach_restart will be automatically retried, but cockroach_restart will behave more like Postgres-style transactions?

This driver currently only uses savepoints to emulate nested transactions - transactions started from Connection::transaction are "normal" BEGIN/COMMIT style transactions. I guess the proposal here would be to add a function like fn savepoint(&self, name: &str) -> Result<Transaction> to Connection?

On a side note, why not use the standard 40001: serialization_failure SQLSTATE as opposed to CR0000?

[nvb]

I may be misinterpreting what you are saying here, so catch me if I am! We still only support savepoints within a transaction (this is clearest is the C++ example and in our Go txn wrapper), so I don't think the proposal would be to add anything to Connection. However, we use the concept of nested transactions to allow retryable transaction loops with priorities that increase when they encounter serialization errors, all while staying within the context of the outer "real" transaction. We have a page written up about this design in our docs.

I talked to the developer who made the decision to create the Cockroach extension error CR0000 for this, and he stated that he simply did not know about serialization_failure. It sounds like switching over to the standard Postgres error code is the right thing to do, so I'm going to look into making this change later this week. Thanks for pointing it out!

[sfackler]

Ah, sorry, I missed the autocommit=False line in the Python example. Adding Transaction::savepoint(&self, name: &str) seems like a good way to handle this.

[nvb]

Yeah that seems like a good approach, as it will permit this use case and others, without making the driver opinionated about how savepoints should be used. How are you thinking of exposing the RELEASE/ROLLBACK semantics? Should they be tied to the lifetime of an object like they are with nested transactions?

[sfackler]

I'd imagine savepoint would return a Transaction object like transaction itself does. The logic would probably look something like

fn run_retriable_transaction<T, F>(transaction: &Transaction, mut f: F) -> Result<T, Error>
    where F: FnMut(&Transaction) -> T
{
    loop {
        let sp = try!(transaction.savepoint("cockroach_retry"));
        match f(&sp).and_then(|t| sp.commit().map(|_| t)) {
            Err(ref e) if e.code == SqlState::SerializationFailure => {}
            r => return r
        }
    }
}

run_retriable_transaction(&t, |t| {
    try!(t.execute("INSERT INTO foo ...."));
    ....
});