Implications of read-only transactions on Jakarta Connectors specification

[tadamski]

It has recently been decided that Jakarta Transaction specification would allow for marking the transaction as read-only one. See full discussion here:
jakartaee/transactions#220

I believe this feature may have implications on Connectors specification. There are two modes on transaction operation and both of them may be impacted:

• transaction across multiple resource managers (8.2.1) - in this case application server would have to make sure that read-only contract is being held f.e. by making sure that there is an attempt to put a resource into read only mode during enlistment and notifying transaction manager if this attempt failed. On one hand, this contract is described in Transaction specification but it may be hinted here as well as one of the sub-points to emphasize that read-only behavior has to be supported.
• local transaction (8.2.2) - local transaction doesn't rely on Transactional annotation, so it was supposed to support read-only hint it has to be added to Connector interfaces. It seems to me that this feature may be useful here as well. Simple, one database configurations, may often use local transaction configuration and JPA specific mechanism mentioned in transaction discussion attached above (f.e. lack of dirty tracking) may be beneficial.

[tomjenkinson]

For a bit of information, in the XAResource case, if the resource manager does not support readonly it will be asked to rollback and thus should not cause an unexpected failure in terms of potentially written data becoming visible outside of the transaction: https://github.com/jakartaee/transactions/pull/222/files#diff-a7f1a8f7aa4fe67e98dd606d2752e6380bbc1d952dbd3e46907126923ba6125eR420-R423

[njr-11]

For a bit of information, in the XAResource case, if the resource manager does not support readonly it will be asked to rollback and thus should not cause an unexpected failure in terms of potentially written data becoming visible outside of the transaction: https://github.com/jakartaee/transactions/pull/222/files#diff-a7f1a8f7aa4fe67e98dd606d2752e6380bbc1d952dbd3e46907126923ba6125eR420-R423

If I understand correctly, you are planning to allow an XAResource that is not capable of read-only to enlist into a transaction that is marked as read-only, allow the application to proceed with mistakenly making updates to the non-read-only XAResource under the transaction, and then, when the application commits the transaction, you will roll back changes that were made to the non-read-only XAResource, resolve the other participating XAResources, and consider the transaction successfully completed. Why would an application want that behavior? If an application thinks it is updating something but is not actually updating it because it is read only, that should be surfaced as an error so that the application can confront its wrong expectations rather than being hidden so that the application believes that it has successfully committed the updates it believes it made.

[beikov]

Why would an application want that behavior?

Because they want the downstream improvements that can be achieved by marking a transaction as read-only i.e. avoid dirty tracking etc.

Not every resource that someone wants to leverage might have the same level of read-only understanding. For some databases, setting a connection to read-only is just a hint to avoid some heavy lifting upfront, but it might still be possible to invoke mutation operations. For those resources that have a strict understanding i.e. that any data mutations are illegal, we have this paragraph:

A read-only transaction may fail early if a non-read action is executed.

A JTA implementation is free to provide a configuration option that fails a read-only transaction immediately when trying to enlist an XAResource that doesn't support a read-only mode. This was also brought up in the discussion, but AFAIR, the conclusion was that this sort of configuration would be an implementation specific thing, nothing that should be mentioned in the spec: jakartaee/transactions#222 (comment)

If an application thinks it is updating something but is not actually updating it because it is read only, that should be surfaced as an error so that the application can confront its wrong expectations rather than being hidden so that the application believes that it has successfully committed the updates it believes it made.

Like I already tried to convey, not every resource has the same understanding of read-only. It would be a pity if applications that behave well (and don't mutate data) couldn't benefit from the positive effects of marking a transaction read-only. In the end, it is up to the XAResource to throw errors early when mutation operations are invoked. Imposing this requirement to throw an error when non-read-only resources are enlisted would IMO make the feature only half as useful.

I also doubt that the read-only transactions in Spring have stronger guarantees.

[njr-11]

it is up to the XAResource to throw errors early when mutation operations are invoked

That would be an excellent solution if the specification or Javadoc actually required an error on attempted write during read-only. Absent that or the other approach I suggested, you are designing a solution where applications are committing updates that they believe will go into the database but actually disappear. Correctness of the application's data is more important than optimizing performance.

The introduction of a special ExtendedXAResource in Jakarta EE creates a disconnect between JCA connection management and the actual XAResource implementation that comes from a JDBC driver or resource adapter, with the inability for Jakarta EE to mandate needed requirements against providers outside of Jakarta EE that implement a Java SE interface.

I think the right way to go about this would be to propose the requirement in Java SE for XAResource itself to have the setReadOnly method along with the contract that resource operating in read only mode must raise an error if a write operation is attempted. Any chance the Jakarta Transaction spec group can be convinced to go with that approach instead?

[beikov]

The Javadoc of the @Transactional annotation says

A read-only transaction may fail early if a non-read action is executed.

A requirement for an error is going to force implementations to either support a strict read-only mode on the "backend" so that it can throw an error when mutations happen, or the client to understand all operations completely. Since e.g. custom SQL functions (aka. UDF) could mutate state, knowing this for sure on the client side is almost impossible or very costly.

This in turn means that users of databases/resources that don't support this strict read-only mode would have to give up on all the potential optimizations that downstream libraries could leverage. Furthermore, it would harm database portability, because if you want to switch from one database to another e.g. for testing vs. production, you might have the problem that one of the two environments doesn't support read-only. Even though you know your application behaves fine and doesn't do mutations, you can't have any benefits. To me, this sounds like a worse outcome.

Absent that or the other approach I suggested, you are designing a solution where applications are committing updates that they believe will go into the database but actually disappear. Correctness of the application's data is more important than optimizing performance.

But why would a correct application even do mutation operations in a read-only transaction? Any expectation about state changes in such a scenario are "bugs". At best, people will use JPA or Jakarta Data which has an understanding of mutation operations and can throw the errors already. For the people that use JDBC directly, they are indeed at the mercy of the driver support, but I don't see a problem with correctness.

[njr-11]

But why would a correct application even do mutation operations in a read-only transaction?

My concern isn't about correct applications here. The concern is about introducing a design that leads applications to unknowingly write bad code and lose updates to their data as a consequence. Maybe it will help to write out an example.

Here is a very common pattern where applications use application managed transactions,

    void doSomething() throws Exception {
        UserTransaction tran = InitialContext.doLookup("java:comp/UserTransaction");
        tran.begin();
        // perform transaction operations, including updates
        tran.commit();
    }

    void anotherMethod() throws Exception {
        ...
        doSomething();
    }

Now, let's add in usage of the new read only pattern,

    void doSomethingElse() throws Exception {
        UserTransaction tran = InitialContext.doLookup("java:comp/UserTransaction");
        tran.setReadOnly(true);
        tran.begin();
        // perform read operations only
        tran.commit();
    }

    void anotherMethod() throws Exception {
        doSomethingElse();
        ...
        doSomething();
    }

This causes updates that are made by the previously existing doSomething method that used to work fine to now be rolled back with the application completely unaware of it.

What I'm pointing out here is that the design for read only transactions is highly error prone and produces the sort of errors that result in lost data rather than exceptions or messages. It does not seem responsible of us to introduce a design like that.

There are multiple options to improve the design to eliminate its vulnerability to errors of this kind. They all have trade offs of some sort. Here is another option: make tran.commit() raise an error when in read only mode, forcing the application to use tran.rollback() for the read only pattern. That would avoid the vulnerability here because when the application makes the mistake above it will see an error on the commit attempt and be notified rather than having no awareness of losing the update to their data.

[beikov]

This causes updates that are made by the previously existing doSomething method that used to work fine to now be rolled back with the application completely unaware of it.

I'm not sure I can follow your train of thought here. Can you please give a more concrete example? What data is lost here when doSomethingElse only does read operations? Are you assuming that anotherMethod runs in a write transaction? If so, then please note that an outer write transaction is suspended when a read transaction begins.

Here is a very common pattern where applications use application managed transactions,

I personally doubt that using a UserTransaction directly is a "common pattern" since the @Transactional annotation is a lot more convenient. For the few users that do use this API, I can see how forcing a user to call rollback() might potentially make the effects clearer. If you want to propose a spec change for that aspect, that would be ok for me.

[njr-11]

What data is lost here when doSomethingElse only does read operations?

Nothing that thedoSomethingElse method does is lost.

Are you assuming that anotherMethod runs in a write transaction? If so, then please note that an outer write transaction is suspended when a read transaction begins.

No. The anotherMethod method doesn't run in any transaction. There is no nesting of transactions here.

I'm not sure I can follow your train of thought here.

The subtle problem here is that the doSomethingElse method creates a side-effect where read-only remains true on the UserTransaction that is associated with the thread. So, when the doSomething method subsequently runs after it, read-only is still true for the transaction. It doesn't matter that the transaction was looked up a second time. It's the same transaction. The doSomething method makes updates under the transaction and believes that it is committing, having received no error, but the update is actually rolled back and lost.

I personally doubt that using a UserTransaction directly is a "common pattern" since the @transactional annotation is a lot more convenient. For the few users that do use this API,

I can say that we see do see a lot of use of UserTransaction (whether looked up or injected - both ways experience the same problem here), as well as of @Transactional. They are both important and widely used.

UserTransaction vs @Transactional is really a question of application managed vs container managed transactions. Some applications will choose application managed. Others will choose container managed.

forcing a user to call rollback() might potentially make the effects clearer. If you want to propose a spec change for that aspect, that would be ok for me.

Yes, I think we should do that. Thanks for discussing to help identify what sort of change would be most acceptable to cover this.

[scottmarlow]

forcing a user to call rollback() might potentially make the effects clearer. If you want to propose a spec change for that aspect, that would be ok for me.

Yes, I think we should do that. Thanks for discussing to help identify what sort of change would be most acceptable to cover this.

Are we thinking that for read-only transactions any calls to commit() should throw an exception? Or something else?

[njr-11]

Are we thinking that for read-only transactions any calls to commit() should throw an exception?

Yes, that's what I was referring to. This solves the problem of transactions that perform updates accidentally running as read-only due to side-effects of prior code. The attempted commit will fail, notifying the application of the error condition rather than hiding it and letting the application think the data was updated.

[beikov]

The subtle problem here is that the doSomethingElse method creates a side-effect where read-only remains true on the UserTransaction that is associated with the thread. So, when the doSomething method subsequently runs after it, read-only is still true for the transaction. It doesn't matter that the transaction was looked up a second time. It's the same transaction. The doSomething method makes updates under the transaction and believes that it is committing, having received no error, but the update is actually rolled back and lost.

Well, good that you made this point clear. Maybe the setReadOnly(true) call shall only affect the transaction that is started next i.e. after a begin(), the read-only state is reset to false. That would indeed prevent such problems.

How do others think of such a change?

[scottmarlow]

Are we thinking that for read-only transactions any calls to commit() should throw an exception?

Yes, that's what I was referring to. This solves the problem of transactions that perform updates accidentally running as read-only due to side-effects of prior code. The attempted commit will fail, notifying the application of the error condition rather than hiding it and letting the application think the data was updated.

The subtle problem here is that the doSomethingElse method creates a side-effect where read-only remains true on the UserTransaction that is associated with the thread. So, when the doSomething method subsequently runs after it, read-only is still true for the transaction. It doesn't matter that the transaction was looked up a second time. It's the same transaction. The doSomething method makes updates under the transaction and believes that it is committing, having received no error, but the update is actually rolled back and lost.

Well, good that you made this point clear. Maybe the setReadOnly(true) call shall only affect the transaction that is started next i.e. after a begin(), the read-only state is reset to false. That would indeed prevent such problems.

How do others think of such a change?

Any volunteer to open an issue or issues for both of the above suggestions?

[njr-11]

Any volunteer to open an issue or issues for both of the above suggestions?

I think a single issue would be best rather than trying to write up two different solutions, one of which won't be used. I'd probably start out the single issue by pointing at the conversation here and give the transactions spec group some time to add what discussion they want before they decide on which solution they prefer and go from there. The solution of failing the commit is more complete because it would also eliminate some additional, but less likely, scenarios (such as where prior code sets read-only but for whatever reason opts not to start a transaction and then a subsequent method running on the thread accidentally ends up as read only). But both options address the more important core scenario that is discussed above that I would expect many applications to run into.