Add a io.r2dbc.spi.Savepoint type to allow for unnamed savepoints

[lukaseder]

As a frequent user of JDBC, I find JDBC's ability of creating unnamed savepoints very convenient. This seems to be lacking in the current R2DBC SPI. I would expect the following Connection API (removed Javadoc and other methods for simplicity):

public interface Connection {
    Publisher<Savepoint> createSavepoint();
    Publisher<Savepoint> createSavepoint(String name);
    Publisher<Void> releaseSavepoint(Savepoint savepoint);
    Publisher<Void> rollbackTransaction();
    Publisher<Void> rollbackTransactionToSavepoint(Savepoint savepoint);
}

---

See also this discussion:
https://groups.google.com/forum/#!topic/r2dbc/QZpTpQtj1HA

[mp911de]

Thanks a lot.

How about moving Savepoint.release() into the savepoint type itself?

public interface Connection {
    Publisher<Savepoint> createSavepoint();
    Publisher<Savepoint> createSavepoint(String name);
    Publisher<Void> rollbackTransaction();
    Publisher<Void> rollbackTransaction(Savepoint savepoint);
}

interface Savepoint {
    Publisher<Void> release();
}

Not sure we need to expose additional metadata on Savepoint itself.

[lukaseder]

I'm undecided about Savepoint.release(). It seems to make sense, but makes it easier to overlook as it introduces some irregularity in the API, perhaps?

[gregturn]

I think the answer lies in a use case. Try typing a pseudo transaction using both ways and see how it reads.

I understand the desire to move release into Savepoint, but the intention might read better if left in Connection.

[nebhale]

@lukaseder So what does the implementation of unnamed savepoints do? Does it generate a random name and then use the Savepoint type as a holder for this?

[lukaseder]

That's certainly what many implementations do, or rather than random, they use a sequence. But I would say that the behaviour is implementation specific.

[mp911de]

FWIW, SQL Server JDBC and Posgres drivers use a sequence (counter) per connection.

[nebhale]

And the Savepoint is a marker interface for driver-specific types? The expectation being that it’ll need to be downcasted and an exception thrown if it’s not the implementation that the driver understands?

[lukaseder]

Well, the JDBC Savepoint type has two methods: getSavepointId() and getSavepointName(). So, drivers could use those methods.

[gregturn]

A) is there no risk of mixing identifiers between databases?

B) how do ensure integrity if you’re in a cloud native environment running 10,000 instances?

I pick 10,000 as being Netflix scale. And something that is unlikely to happen when you run two copies becomes much more likely when running at Netflix scale.

[lukaseder]

I would say that the identifiers are connection / transaction scoped, so there shouldn't be any risk, no?

[ttddyy]

Just FYI, I found there is SavepointManager in spring (since 1.1 :o), and it defines only one method for creating savepoint:

Object createSavepoint() throws TransactionException;

The actual implementation uses sequential number in ConnectionHolder#createSavepoint:

public Savepoint createSavepoint() throws SQLException {
  this.savepointCounter++;
  return getConnection().setSavepoint(SAVEPOINT_NAME_PREFIX + this.savepointCounter);
}

According to the javadoc:

This interface is inspired by JDBC 3.0's Savepoint mechanism
but is independent from any specific persistence technology.

So, it is the savepoint abstraction in spring.

[nebhale]

Building on @ttddyy's research into Spring, @lukaseder why should this be part of the R2DBC driver implementations and not implemented as a purely client behavior? Auto-generation of ids seems to be something that all drivers would have to replicate more-or-less similarly which makes it feel like it should exist at a higher level.

[nebhale]

I think the problem with this change lies here. In a reactive flow like this, there's no obvious way to maintain the state that is returned from createSavepoint(). The clarity of this code (which you might disagree with 😉 ) builds on the fact that only results flow down. All control behaviors are Publisher<Void> so they can complete without interfering in the flow of results.

[lukaseder]

why should this be part of the R2DBC driver implementations and not implemented as a purely client behavior

Call me old fashioned, but stringly typed things are not cool. :-) Also, in 99% of all cases I've ever needed a savepoint, I didn't need to name them.

In a reactive flow like this, there's no obvious way to maintain the state that is returned from createSavepoint().

I see this point, but the identification via string name still feels a bit wrong to me

[mp911de]

Considering that R2DBC SPI isn't really meant for end-users, savepoint management would fall into a client's responsibility – basically the same as for transaction management. Drawing some pseudo-code how a client library could deal with save points:

client.inTransaction(handle -> {

  return handle.insert(…)
    .then(handle.update(…))
    .then(handle.createSavepoint())
    .then(handle.doSomethingExceptional())
    .onError(handle.getLastSavepoint().flatMap(handle::rollback));
});

Although we're talking reactive/potentially async from an API perspective, we still need to consider that the majority (not sure if all) of databases are bound to sequential execution and impose severe limitations such as binding to a single transaction, a single result set.