sql: queries with ON CONFLICT DO UPDATE SET seem to interfere each other when run in parallel

[ZhouXing19]

Describe the problem

We spinned up 2 threads to run the same INSERT INTO ... ON CONFLICT DO UPDATE SET ... query in parallel. To do that we started a connection pool with maximum pool size == 2 connections. When running against the latest built from master we encountered the following error:

org.postgresql.util.PSQLException: ERROR: there is already a transaction in progress
...

This seems to be a regression caused by commit 9fdb39b49e. It passed with a built prior to this commit.

To Reproduce

We used HikariCP 5.0.0 as the connection pool, and run the query with the following insertSameValInParallel.java.

package jane.demo.connect;

import com.zaxxer.hikari.HikariDataSource;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;


public class insertSameValInParallel {
    static HikariDataSource hds = new HikariDataSource();

    public static class incrementInsert implements Runnable {
        private final int threadId;
        public incrementInsert(int threadId) {
            this.threadId = threadId;
        }
        public void run() {
            int valToInsert = 0;
            while (true) {
                try {
                    insertVal(valToInsert);
                    valToInsert += 1;
                    System.out.printf("Done inserting %d with thread %d\n", valToInsert, this.threadId);
                } catch (Exception e) {
                    e.printStackTrace();
                    System.exit(1);
                }
            }
        }
    }

    public static void main(String[] args) throws Exception {
        hds.setDriverClassName(org.postgresql.Driver.class.getName());
        hds.setJdbcUrl("jdbc:postgresql://localhost:26257/defaultdb?sslmode=disable");
        hds.setUsername("root");
        // If set maximum pool size to 1, no error.
        hds.setMaximumPoolSize(2);
        hds.setAutoCommit(true);

        initTable();

        // Start 2 thread, each will try to insert incremental values to the same table.
        for (int threadId = 0; threadId < 2; threadId++) {
            Thread t = new Thread(new incrementInsert(threadId));
            t.start();
        }

        while (true) Thread.sleep(1000);
    }

    private static void initTable() throws SQLException {
        try (Connection conn = hds.getConnection()) {
            String dropTableSql = "DROP TABLE IF EXISTS public.t; ";
            String createTableSql = "CREATE TABLE public.t (x int, CONSTRAINT my_pkey PRIMARY KEY (x ASC));";
            PreparedStatement stmt = conn.prepareStatement(dropTableSql + createTableSql);
            stmt.executeUpdate();
        }
    }

    private static Object[] insertVal(int val) throws SQLException {
        try (Connection conn = hds.getConnection()) {
            try {
                conn.setAutoCommit(false);
                String sql = String.format(
                            "INSERT INTO t (x) VALUES (%d) " +
                            "ON CONFLICT (x) DO UPDATE SET x = 1000 + %d",
                        val,
                        val
                );
                PreparedStatement stmt = conn.prepareStatement(sql);
                stmt.executeUpdate();
            } finally {
                conn.setAutoCommit(true);
            }
        }
        return null;
    }
}

We spinned up a cockroach server with ./cockroach start-single-node --insecure --store=cockroach-data --advertise-addr=localhost.

We tested against the binary built at the

a) current latest commit in master (2181204e9c)

b) 9fdb39b49e

c) e852be5ca4 (the newest commit prior to #9fdb39b49e)

a) and b) failed with the org.postgresql.util.PSQLException: ERROR: there is already a transaction in progress error and c) passed

Note that any of following changes in insertSameValInParallel.java will make it pass:

• Set the maximum pool size to 1.
• Start only one thread.
• Comment out the conn.setAutoCommit(false); in insertVal().
• Change the query to ON CONFLICT DO NOTHING.
• Add a sleep between thread starts. For example: (Note that the sleep time has to be longer than 100 milliseconds)

      for (int threadId = 0; threadId < 2; threadId++) {
         Thread t = new Thread(new incrementInsert(threadId));
         if (threadId == 1) {
             Thread.sleep(100);
         }
         t.start();
     }

Expected behavior
The queries should not fail.

Additional data / screenshots
The whole error message is

org.postgresql.util.PSQLException: ERROR: there is already a transaction in progress
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2674)
	at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2364)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:354)
	at org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:484)
	at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:404)
	at org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:162)
	at org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:130)
	at com.zaxxer.hikari.pool.ProxyPreparedStatement.executeUpdate(ProxyPreparedStatement.java:61)
	at com.zaxxer.hikari.pool.HikariProxyPreparedStatement.executeUpdate(HikariProxyPreparedStatement.java)
	at jane.demo.connect.insertSameValInParallel.insertVal(myClass.java:72)
	at jane.demo.connect.insertSameValInParallel$incrementInsert.run(myClass.java:22)
	at java.base/java.lang.Thread.run(Thread.java:832)

Environment:

• Server OS: [MacOS]
• Client app [JDBC]
• Cockroach Server: single-node insecure node, initialized with ./cockroach start-single-node --insecure

Additional context
This issue is originally from a support ticket.

Jira issue: CRDB-16346

gz#12585

[ZhouXing19]

NB: The error seems to be more related to two connections trying to commit at the same time. I tried turning off the auto commit and manually added conn.commit() after executing each query, and the error persists. However, if I set the auto commit to be true at the very beginning, the error is gone.

[rafiss]

I played with this example a little bit more. The issue also only happens if the two threads are trying to write the same values. (I changed it so one of the threads had an initial valToInsert of 9999 and the bug did not occur.) So it might relate to transaction contention.

[rafiss]

I added additional logging to e852be5ca4 (the working commit).

session 6074260520113121 prepare BEGIN
session 7161210049600088 prepare BEGIN
session 6074260520113121 execNotxn BEGIN TRANSACTION
session 6074260520113121 do BEGIN
session 6074260520113121 prepare INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING
session 7161210049600088 execNotxn BEGIN TRANSACTION
session 7161210049600088 do BEGIN
session 7161210049600088 prepare INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING
session 6074260520113121 execOpen INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING
session 7161210049600088 execOpen INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING
session 6074260520113121 prepare COMMIT
session 6074260520113121 execOpen COMMIT
session 6074260520113121 do COMMIT
session 6074260520113121 prepare BEGIN
session 6074260520113121 execNotxn BEGIN TRANSACTION
session 6074260520113121 do BEGIN
session 6074260520113121 prepare INSERT INTO t (x) VALUES (1) ON CONFLICT (x) DO NOTHING
session 6074260520113121 execOpen INSERT INTO t (x) VALUES (1) ON CONFLICT (x) DO NOTHING
session 6074260520113121 execOpen COMMIT
session 6074260520113121 do COMMIT
session 7161210049600088 execOpen INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING
session 7161210049600088 prepare COMMIT
session 7161210049600088 execOpen COMMIT
session 7161210049600088 do COMMIT
session 7161210049600088 prepare BEGIN
session 7161210049600088 execNotxn BEGIN TRANSACTION
session 7161210049600088 do BEGIN
session 7161210049600088 prepare INSERT INTO t (x) VALUES (1) ON CONFLICT (x) DO NOTHING
session 7161210049600088 execOpen INSERT INTO t (x) VALUES (1) ON CONFLICT (x) DO NOTHING

Note that session ID 7161210049600088 has two lines with execOpen INSERT INTO t (x) VALUES (0) ON CONFLICT (x) DO NOTHING. I think that means that CRDB is automatically retrying the statement (that's actually a bit surprising to me, since I thought that would only happen for implicit transactions).

A theory I have is that after 9fdb39b, CRDB is still trying to retry the statement, but it also retries the BEGIN. I'll try testing again with logs on that commit to confirm.

[rafiss]

#82622 fixes this, but I need to figure out how to write a unit test for it.