MySQL Bugs: #104067: No reset autoCommit after unknown issue occurs

Bug #104067	No reset autoCommit after unknown issue occurs
Submitted:	21 Jun 2021 8:47	Modified:	3 Dec 2021 19:58
Reporter:	Tingyu Wei (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector / J	Severity:	S3 (Non-critical)
Version:	8.0.25	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
If an unknown exception other than a network exception occurs when executing setAutoCommit SQL, the autoCommit value of server session should be reset to the previous value.

Otherwise, the connection was picked by other threads, and the autoCommit value of server session is not equals to the value of autoCommit from DB, which cause the transaction enter to a wrong status after doBegin: The app server is autoCommit = false, and the DB server id autoCommit = true. In this case, the DB wouldn't exec SQLs in the method as a transaction.

How to repeat:
1. Start a HTTP service with a transaction API, and the service use the following frameworks: 
  a. mysql-connector-j
  d. HikariCP:2.7.6
  e. spring-framework:4.3.18.RELEASE
2. Mysql DB Server throw a unknown issue if App server exec the SQL: "set autoCommit = false"

3. we can see the current thread can throw an exception. 
In the case, the autoCommit value is true in DB connector. while the autoCommit value in the connector session still keep false. 
And the next thread who get the DB connector will inherit the autoCommit = false. 

4. The result: The transactional of the next thread has not take effective in DB.

Suggested fix:
Reset the autoCommit value if unknown issue occurs when setting autoCommit.
And here is my PR, please help to review, thanks.
https://github.com/mysql/mysql-connector-j/pull/66

Hi,

I'm verifying this as FR, not as a bug. Thanks for the report and for the patch.

all best
Bogdan Kecman

Dear MySQL Verification Team,
Please double check the case and reopen it.

This case is indeed a bug. 
If the DB connection set autocommit fails, and I use a connection pool, the next thread will get the error autocommit value from connection in pool locally.

This has caused transaction failures and business losses in our company's business.

Expected: if set autocommit fails, the local autocommit should be reset to old value. Pls ref the link: https://github.com/mysql/mysql-connector-j/pull/66/files

Thanks a ton!

Hi,

Bug is now in the hands of Connector/J development team. There is no "reopening" as bug is already in "Verified", from there we are now waiting for "Closed" when the fix is implemented

Hi MySQL Verification Team,

Could you please tell me how the Connector/J development team will handle the bug?
And Will my PR be accepted?

Best Regards

Contribution submitted via Github - reset autoCommit after unknown issue occurs (*) Contribution by tingyu wei (Github weitingyuk, mysql-connector-j/pull/66#issuecomment-867643652): @mysql-oca-bot I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it. Here is my bug report. please help to review. https://bugs.mysql.com/bug.php?id=104067&thanks=4

Contribution: git_patch_663614465.txt (text/plain), 1.93 KiB.

Dear OCAAdmin,
Thanks for attach my contribution, I am not sure what I need to do in the next step for fixing the bug.

If any question, pls connect me any time.

Thanks.

Hi MySQL Verification Team,

Could you please kindly tell me when the bug fix will be merged in to the master branch? since out company looking forward to use the new feature.

Thanks very much.
Best Regards.

By Tingyu

Hi Tingyu,

Thank you for your interest in MySQL and Connector/J.

Please help me better understand your problem: What exceptions are you expecting from executing a statement like `set autoCommit = false`? In what situation is this throwing an exception that is not network/connectivity issue or something that entirely invalidates the connection object?

Can you please provide a test case using just Connector/J?

No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".

Hi Filipe,
Thanks very much for your response, and here is my feedback, sorry for the late.
Please kindly help to review.

Q1: What exceptions are you expecting from executing a statement like `set autoCommit = false`? 

A1 : `set autoCommit = false` can throw an unknown exception in our case, which is a proxy enhancement from our company. And I believe other customers also may extend the exception.
And here is a bug need to be fixed.

Q2: In what situation is this throwing an exception that is not network/connectivity issue or something that entirely invalidates the connection object?

A2: The situation was customized by our proxy, if DB traffic exceed the limitation, the proxy will throw an unknown exception. In this case, the exception from DB is not a network/connectivity issue, but an unknown exception.

Q3: Can you please provide a test case using just Connector/J?

A3:  Sure, here is the pseudocode  for mock testCase,  since I can’t mock the exception, please help to check it.

@Test
public void testBug104067() throws Exception {
    Connection mockConnection = null;
    Properties props = new Properties();
    try {
        mockConnection = getConnectionWithProps(props); // creates a new connection

        CJException cjException = new CJException("Traffic exceeded limitation");
        cjException.setVendorCode(ER_UNKNOWN_ERROR);

        /* ------- here is a mock code -------------*/
        when(((ConnectionImpl)mockConnection).getSession().execSQL(null,  "SET autocommit=0", -1, null, false, Any(ResultSetFactory.class),
                        null, false)).thenThrow(cjException);

        /* ------- the exec will throw an CJException -------------*/
        mockConnection.setAutoCommit(false);
    } finally {

        /* ------- the autocommit was false, which is unexpected result -------------*/
        assertFalse(mockConnection.getAutoCommit());

        if (mockConnection != null) {
            mockConnection.close();
        }
    }
}

Please feel free to ping me if any thing wrong.I am very sorry. Since I can't mock the exception when exec the SQL, the test case just a pseudo code.
Thanks a ton.

Hi Tingyu,

From what I understand these exceptions your proxy may cause can happen anytime, anywhere, right? If so, I'd say that there are many other situations that can lead to inconsistencies between client-side session state tracking and server-side session states. Are you OK with that?

Your patch doesn't seem harmful in any way but maybe I would skip checking the vendor error code (ER_UNKNOWN_ERROR == sqlE.getVendorCode()) you suggested.

I came up with the following test case. Does it reproduce accurately this behavior you are describing?

import java.sql.Connection;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Properties;
import java.util.function.Supplier;

import org.junit.jupiter.api.Test;

import com.mysql.cj.Query;
import com.mysql.cj.conf.PropertyKey;
import com.mysql.cj.exceptions.ExceptionFactory;
import com.mysql.cj.exceptions.MysqlErrorNumbers;
import com.mysql.cj.protocol.Resultset;

import testsuite.BaseQueryInterceptor;
import testsuite.BaseTestCase;

public class Bug104067 extends BaseTestCase {
    @Test
    public void testBug104067() throws Exception {
        Properties props = new Properties();
        props.setProperty(PropertyKey.queryInterceptors.getKeyName(), Bug104067QueryInterceptor.class.getName());
        Connection testConn = getConnectionWithProps(props);
        Statement testStmt = testConn.createStatement();

        System.out.println("Default autocommit value:");
        System.out.println("  Connection autocommit = " + testConn.getAutoCommit());
        rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
        rs.next();
        System.out.println("  Server session autocommit = " + rs.getString(2).equalsIgnoreCase("ON"));

        System.out.println("---");
        System.out.println("After conn.setAutcommit(true):");
        try {
            testConn.setAutoCommit(true);
        } catch (SQLException e) {
            System.out.println("  !!! Exception: " + e.getMessage());
        }
        System.out.println("  Connection autocommit = " + testConn.getAutoCommit());
        rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
        rs.next();
        System.out.println("  Server session autocommit = " + rs.getString(2).equalsIgnoreCase("ON"));

        System.out.println("---");
        System.out.println("After conn.setAutcommit(false):");
        try {
            testConn.setAutoCommit(false);
        } catch (SQLException e) {
            System.out.println("  !!! Exception: " + e.getMessage());
        }
        System.out.println("  Connection autocommit = " + testConn.getAutoCommit());
        rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
        rs.next();
        System.out.println("  Server session autocommit = " + rs.getString(2).equalsIgnoreCase("ON"));

        System.out.println("---");
        System.out.println("After conn.setAutcommit(true):");
        try {
            testConn.setAutoCommit(true);
        } catch (SQLException e) {
            System.out.println("  !!! Exception: " + e.getMessage());
        }
        System.out.println("  Connection autocommit = " + testConn.getAutoCommit());
        rs = testStmt.executeQuery("SHOW SESSION VARIABLES LIKE 'autocommit'");
        rs.next();
        System.out.println("  Server session autocommit = " + rs.getString(2).equalsIgnoreCase("ON"));
        
    }

    public static class Bug104067QueryInterceptor extends BaseQueryInterceptor {
        @Override
        public <T extends Resultset> T preProcess(Supplier<String> str, Query interceptedQuery) {
            String sql = str.get();
            if (sql.equalsIgnoreCase("SET autocommit=0")) {
                throw ExceptionFactory.createException("Some non-connection related error while executing \"" + sql + "\"");
            }
            return super.preProcess(str, interceptedQuery);
        }
    }
}

This is the output I get from it:

Default autocommit value:
  Connection autocommit = true
  Server session autocommit = true
---
After conn.setAutcommit(true):
  Connection autocommit = true
  Server session autocommit = true
---
After conn.setAutcommit(false):
  !!! Exception: Some non-connection related error while executing "SET autocommit=0"
  Connection autocommit = false
  Server session autocommit = true
---
After conn.setAutcommit(true):
  Connection autocommit = true
  Server session autocommit = true

Hello, thank you for your detailed reply, and the test case seems very good,  give your job thumbs up.

The reasons why I added these two conditions are as follows. These are just my personal idea for reference only:

1. !(sqlE instanceof CJCommunicationsException)  since it is possible after MySQL sever connection has set auto commit successfully and occurs the network issue when returns the result.  At this time, it cannot be set to the original value. Moreover, the underlying layer has been processed. If autoreconnect = true, it may enter 2039 lines of code of NativeSession.java and execute the following code” this.protocol.getSocketConnection().forceClose(); maybe this case will cause a NPE issue.

2. ER_UNKNOWN_ERROR == sqlE.getVendorCode()
There are many codes in MysqlErrorNumbers.java.  I really don't know much about other codes except ER_UNKNOWN_ERROR, If all numbers other than error are reset directly, will there be new problems? I'm not sure, so I added this ER_ UNKNOWN_ Error condition. 

Please correct me immediately if there is something wrong with my idea.

Thanks very much
By Tingyu Wei

Alright. I think we are OK with accepting your contribution. We may change it a little bit but in the end it should do what you expect.

Thank you again.

Got it, thanks very much.
Could you please kindly inform me if the PR was merged into master?

Thanks a ton.
By TingyuWei

Hi Tingyu Wei,

I'm sorry, that information cannot be disclosed as per Oracle policies. You'll know the patch is merged after the release that includes it is published. I can't even predict in what release it will be included.

Best regards,
Filipe

Posted by developer:
 
Added the following entry to the Connector/J 8.0.28 changelog: 

"Some Java frameworks prevent changes of the autocommit value on the MySQL Server. When an application tried to change autocommit, the value was changed for the connection Session on the client side, but it failed to change on the server, so the settings became out of sync. With this fix, Connector/J resets autocommit to its original value for the Session under the situation. Thanks to Tingyu Wei for contributing to the fix."