Prevent Oracle from committing uncommitted transactions on connection close

[gsmet]

When a connection is closed, Oracle will commit the unfinished business, which is a very surprising behavior in most cases and could lead to data loss.

I added a -Dquarkus-oracle-no-automatic-rollback-on-connection-close to disable this behavior if absolutely needed.

If we have feedback that this is not something we should do always, we can add a proper configuration property (with a default value to determine).

Fixes #36265

Wouldn't have been possible without the nice work of @segersb and @Felk.

[github-actions]

🙈 The PR is closed and the preview is expired.

[gsmet]

Let's make sure @yrodiere is on board with this before merging. It obviously will require a note in the migration guide.

[yrodiere]

Thanks for taking care of this.

LGTM, just a few comments that can probably be dismissed if you disagree.

[segersb]

Hey guys, I saw the comment about XA connections and did some low level testing.
I get the same strange behavior, data gets committed without an explicit commit.

Here's the setup

import oracle.jdbc.xa.client.OracleXADataSource;
 
import javax.sql.XAConnection;
import java.sql.Connection;
import java.sql.PreparedStatement;
 
public class TextXA {
    public static void main(String[] args) throws Exception {
        // 1. Set up the XA DataSource
        OracleXADataSource xaDataSource = new OracleXADataSource();
 
        // adjust to your DB
        xaDataSource.setURL("");
        xaDataSource.setUser("");
        xaDataSource.setPassword("");
 
        // 2. Get the XAConnection
        XAConnection xaConn = null;
        try {
            xaConn = xaDataSource.getXAConnection();
            // 3. Get the physical JDBC Connection
            try (Connection conn = xaConn.getConnection()) {
                // Set auto-commit mode = false to be sure
                conn.setAutoCommit(false);
 
                // 4. Insert some data
                try (PreparedStatement pstmt = conn.prepareStatement("insert into D00SREFTPB01.test values (1234)")) {
                    pstmt.executeUpdate();
                }
 
                // 5. No commit
            }
        } finally {
            if (xaConn != null) {
                // 6. Close the XAConnection
                xaConn.close();
            }
        }
    }
}

My understanding of the issue is that the JTA transaction manager never gets the chance to either commit or roll back the transaction.

In an XA scenario I believe that the transaction manager needs to either prepare+commit or rollback multiple connections.
So I would expect that the same issue will be happening when using XA as well.
Just that partial work would be committed over multiple connections.

[gsmet]

@segersb I'm unsure how this is supposed to happen, I would have expected the XA layer to handle things properly in this case but I might be mistaken.

What bugged me at the beginning was the Javadoc for rollback():

     * @throws SQLException if a database access error occurs,
     * this method is called while participating in a distributed transaction,
     * this method is called on a closed connection or this
     *            {@code Connection} object is in auto-commit mode

The while participating in a distributed transaction section in particular.

[segersb]

@gsmet, I did some additional testing and digging into XA.

In my previous example I was using an XA data source, but I was misusing it as a "regular" connection.

When using the proper way to initiate an XA transaction, Oracle seems to behaves as we would expect:
there is no auto-commit on close, and I only see the inserted data after performing an explicit commit.

Also, as mentioned in the javadoc you referenced, an exception is thrown when rolling back an underlying connection.
The global XA transactions can only be rolled back via the XA resource.

Therefore, ignoring XA connections in the rollback interceptor is indeed the appropriate solution.

    public static void main(String[] args) throws Exception {
        // Set up the XA DataSource, adjust to your DB
        OracleXADataSource xaDataSource = new OracleXADataSource();
        xaDataSource.setURL(URL);
        xaDataSource.setUser(USER_NAME);
        xaDataSource.setPassword(PASSWORD);

        // Create a global distributed transaction ID
        OracleXid xid = new OracleXid(0, new byte[]{0x01}, new byte[]{0x01});

        // Start the global transaction
        XAConnection xaConn = xaDataSource.getXAConnection();
        XAResource xaRes = xaConn.getXAResource();
        xaRes.start(xid, XAResource.TMNOFLAGS);
        
        Connection connection = xaConn.getConnection();

        // No need to set auto-commit to false, it is false by default in a global transaction
        // It cannot even be set to true in a global transaction, it will throw an exception
        
        try (PreparedStatement pstmt = connection.prepareStatement("insert into D00SREFTPB01.test values (1)")) {
            pstmt.executeUpdate();
        }

        // Only when explicitly committing will the data be inserted
//        xaRes.prepare(xid);
//        xaRes.commit(xid, false);

        // Rollback will throw an exception when performed on the underlying connection
//        connection.rollback();
        
        // Rollback of the global transaction is done with the transaction ID
//        xaRes.rollback(xid);

        connection.close();
        xaConn.close();
    }

[gsmet]

@segersb thank you very much for taking the time to experiment with this, it's greatly appreciated!

[gsmet]

I pushed some minor doc adjustments.

If you're all happy with it, we can merge and make it part of 3.18.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 44fd4b3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 44fd4b3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[gsmet]

And merged! Thanks for the awesome collaboration on this, everyone!