Skip to content
Permalink
Browse files

Clean up some tests and fix IsValidTest race condition (#1581)

* test: Add some helpers to TestUtil

Adds some helpers to TestUtil to deal with boiler plate of executinga query
and disgarding the results, fetching the backend process id, terminating a
backend process, and fetching the current transaction state.

* test: Clean up IsValidTest

Cleans up IsValidTest and removes a race condition when checking whether
terminated connections are marked as invalid. Also splits out the transaction
state test into three separate ones to better identify what's being tested.

* test: Change CopyTest to use TestUtil backend pid helpers

This also changes the connection termination to use a non-privileged
connection.

* test: Change ConnectionPoolTest to use backend pid TestUtil helpers

This also changes the pg_terminate_backend(...) call to use a non-privileged connection
as a regular user can terminate their own connection.

* fix: Use super user to terminate backend processes in tests

Older versions of the server (<=9.1) restrict calling pg_terminate_backend(...)
to super users so always use that in test helper.
  • Loading branch information
sehrope authored and davecramer committed Oct 24, 2019
1 parent 0fd4535 commit ad734574726eb0decf5178071c87a1b513e484f2
@@ -6,7 +6,9 @@
package org.postgresql.test;

import org.postgresql.PGProperty;
import org.postgresql.core.BaseConnection;
import org.postgresql.core.ServerVersion;
import org.postgresql.core.TransactionState;
import org.postgresql.core.Version;
import org.postgresql.jdbc.PgConnection;

@@ -621,6 +623,11 @@ public static void assertNumberOfRows(Connection con, String tableName, int expe
}
}

public static void assertTransactionState(String message, Connection con, TransactionState expected) {
TransactionState actual = TestUtil.getTransactionState(con);
Assert.assertEquals(message, expected, actual);
}

/*
* Helper - generates INSERT SQL - very simple
*/
@@ -917,6 +924,73 @@ public static boolean isReplicationSlotActive(Connection connection, String slot
}
}

/**
* Execute a SQL query with a given connection and return whether any rows were
* returned. No column data is fetched.
*/
public static boolean executeQuery(Connection conn, String sql) throws SQLException {
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery(sql);
boolean hasNext = rs.next();
rs.close();
stmt.close();
return hasNext;
}

/**
* Retrieve the backend process id for a given connection.
*/
public static int getBackendPid(Connection conn) throws SQLException {
Statement stmt = conn.createStatement();
ResultSet rs = stmt.executeQuery("SELECT pg_backend_pid()");
rs.next();
int pid = rs.getInt(1);
rs.close();
stmt.close();
return pid;
}

/**
* Executed pg_terminate_backend(...) to terminate the server process for
* a given process id with the given connection.
*/
public static boolean terminateBackend(Connection conn, int backendPid) throws SQLException {
PreparedStatement stmt = conn.prepareStatement("SELECT pg_terminate_backend(?)");
stmt.setInt(1, backendPid);
ResultSet rs = stmt.executeQuery();
rs.next();
boolean wasTerminated = rs.getBoolean(1);
rs.close();
stmt.close();
return wasTerminated;
}

/**
* Create a new connection using the default test credentials and use it to
* attempt to terminate the specified backend process.
*/
public static boolean terminateBackend(int backendPid) throws SQLException {
Connection conn = TestUtil.openPrivilegedDB();
try {
return terminateBackend(conn, backendPid);
} finally {
conn.close();
}
}

/**
* Retrieve the given connection backend process id, then create a new connection
* using the default test credentials and attempt to terminate the process.
*/
public static boolean terminateBackend(Connection conn) throws SQLException {
int pid = getBackendPid(conn);
return terminateBackend(pid);
}

public static TransactionState getTransactionState(Connection conn) {
return ((BaseConnection) conn).getTransactionState();
}

private static void waitStopReplicationSlot(Connection connection, String slotName)
throws InterruptedException, TimeoutException, SQLException {
long startWaitTime = System.currentTimeMillis();
@@ -31,7 +31,6 @@
import java.io.PrintStream;
import java.io.StringReader;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
@@ -353,17 +352,14 @@ public void testLockReleaseOnCancelFailure() throws SQLException, InterruptedExc
// on the Connection object fails to deadlock.
con.setAutoCommit(false);

Statement stmt = con.createStatement();
ResultSet rs = stmt.executeQuery("select pg_backend_pid()");
rs.next();
int pid = rs.getInt(1);
rs.close();
stmt.close();
// We get the process id before the COPY as we cannot run other commands
// on the connection during the COPY operation.
int pid = TestUtil.getBackendPid(con);

CopyManager manager = con.unwrap(PGConnection.class).getCopyAPI();
CopyIn copyIn = manager.copyIn("COPY copytest FROM STDIN with " + copyParams);
try {
killConnection(pid);
TestUtil.terminateBackend(pid);
byte[] bunchOfNulls = ",,\n".getBytes();
while (true) {
copyIn.writeToCopy(bunchOfNulls, 0, bunchOfNulls.length);
@@ -420,23 +416,6 @@ public SQLException exception() {
}
}

private void killConnection(int pid) throws SQLException {
Connection killerCon;
try {
killerCon = TestUtil.openPrivilegedDB();
} catch (Exception e) {
fail("Unable to open secondary connection to terminate copy");
return; // persuade Java killerCon will not be used uninitialized
}
try {
PreparedStatement stmt = killerCon.prepareStatement("select pg_terminate_backend(?)");
stmt.setInt(1, pid);
stmt.execute();
} finally {
killerCon.close();
}
}

private void acceptIOCause(SQLException e) throws SQLException {
if (!(e.getCause() instanceof IOException)) {
throw e;
@@ -9,7 +9,6 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.postgresql.PGConnection;
import org.postgresql.core.ServerVersion;
import org.postgresql.ds.PGConnectionPoolDataSource;
import org.postgresql.jdbc2.optional.ConnectionPool;
@@ -329,18 +328,9 @@ public void testBackendIsClosed() throws Exception {
Assume.assumeTrue("pg_terminate_backend requires PostgreSQL 8.4+",
TestUtil.haveMinimumServerVersion(con, ServerVersion.v8_4));

int pid = ((PGConnection) con).getBackendPID();

Connection adminCon = TestUtil.openPrivilegedDB();
try {
Statement statement = adminCon.createStatement();
statement.executeQuery("SELECT pg_terminate_backend(" + pid + ")");
} finally {
TestUtil.closeDB(adminCon);
}
TestUtil.terminateBackend(con);
try {
Statement statement = con.createStatement();
statement.executeQuery("SELECT 1");
TestUtil.executeQuery(con, "SELECT 1");
fail("The connection should not be opened anymore. An exception was expected");
} catch (SQLException e) {
// this is expected as the connection has been forcibly closed from backend
@@ -5,102 +5,62 @@

package org.postgresql.test.jdbc4;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import org.postgresql.core.BaseConnection;
import org.postgresql.core.ServerVersion;
import org.postgresql.core.TransactionState;
import org.postgresql.test.TestUtil;
import org.postgresql.test.jdbc2.BaseTest4;

import org.junit.Assume;
import org.junit.Test;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.Properties;
import java.sql.SQLException;

public class IsValidTest extends BaseTest4 {

private TransactionState getTransactionState(Connection conn) {
return ((BaseConnection) conn).getTransactionState();
@Test
public void testIsValidShouldNotModifyTransactionStateOutsideTransaction() throws SQLException {
TransactionState initialTransactionState = TestUtil.getTransactionState(con);
assertTrue("Connection should be valid", con.isValid(0));
TestUtil.assertTransactionState("Transaction state should not be modified by non-transactional Connection.isValid(...)", con, initialTransactionState);
}

@Test
public void testIsValid() throws Exception {
try {
assertTrue(con.isValid(0));
} finally {
TestUtil.closeDB(con);
}
assertFalse(con.isValid(0));
public void testIsValidShouldNotModifyTransactionStateInEmptyTransaction() throws SQLException {
con.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
con.setAutoCommit(false);
TransactionState transactionState = TestUtil.getTransactionState(con);
assertTrue("Connection should be valid", con.isValid(0));
TestUtil.assertTransactionState("Transaction state should not be modified by Connection.isValid(...) within an empty transaction", con, transactionState);
}

/**
* Test that the transaction state is left unchanged.
*/
@Test
public void testTransactionState() throws Exception {
TransactionState transactionState = getTransactionState(con);
con.isValid(0);
assertEquals("Transaction state has been changed", transactionState,
getTransactionState(con));

public void testIsValidShouldNotModifyTransactionStateInNonEmptyTransaction() throws SQLException {
con.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
con.setAutoCommit(false);
try {
transactionState = getTransactionState(con);
con.isValid(0);
assertEquals("Transaction state has been changed", transactionState,
getTransactionState(con));

Statement stmt = con.createStatement();
stmt.execute("SELECT 1");
transactionState = getTransactionState(con);
con.isValid(0);
assertEquals("Transaction state has been changed", transactionState,
getTransactionState(con));
} finally {
try {
con.setAutoCommit(true);
} catch (final Exception e) {
}
}
TestUtil.executeQuery(con, "SELECT 1");
TransactionState transactionState = TestUtil.getTransactionState(con);
assertTrue("Connection should be valid", con.isValid(0));
TestUtil.assertTransactionState("Transaction state should not be modified by Connection.isValid(...) within a non-empty transaction", con, transactionState);
}

@Test
public void testIsValidRemoteClose() throws Exception {

assumeMinimumServerVersion("Unable to use pg_terminate_backend before version 8.4", ServerVersion.v8_4);

Properties props = new Properties();
updateProperties(props);
Connection con2 = TestUtil.openPrivilegedDB();

try {
public void testIsValidRemoteClose() throws SQLException, InterruptedException {
Assume.assumeTrue("Unable to use pg_terminate_backend(...) before version 8.4", TestUtil.haveMinimumServerVersion(con, ServerVersion.v8_4));

assertTrue("First connection should be valid", con.isValid(0));
boolean wasTerminated = TestUtil.terminateBackend(con);
assertTrue("The backend should be terminated", wasTerminated);

String pid;
Statement s = con.createStatement();

try {
s.execute("select pg_backend_pid()");
ResultSet rs = s.getResultSet();
rs.next();
pid = rs.getString(1);
} finally {
TestUtil.closeQuietly(s);
// Keeps checking for up to 5-seconds that the connection is marked invalid
for (int i = 0;i < 500;i++) {
if (!con.isValid(0)) {
break;
}

TestUtil.execute("select pg_terminate_backend(" + pid + ")", con2);

assertFalse("The first connection should now be invalid", con.isValid(0));

} finally {
TestUtil.closeQuietly(con2);
// Wait a bit to give the connection a chance to gracefully handle the termination
Thread.sleep(10);
}
assertFalse("The terminated connection should not be valid", con.isValid(0));
}
}

0 comments on commit ad73457

Please sign in to comment.
You can’t perform that action at this time.