diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index 1ec99103..b6ff888f 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -32,7 +32,7 @@ jobs: - name: Install additional build-dependencies for 18+ if: ${{ matrix.pg >= 18 }} - run: apt-get install -y libcurl4-openssl-dev + run: apt-get install -y libcurl4-openssl-dev libnuma-dev - name: Check out the repo uses: actions/checkout@v3 diff --git a/bin/pg_repack.c b/bin/pg_repack.c index b5db6959..4fdb733d 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1799,6 +1799,8 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name) int i; bool ret = true; + pgut_command(conn, "SAVEPOINT repack_sp1", 0, NULL); + initStringInfo(&sql); for (i = 1; ; i++) @@ -1807,8 +1809,6 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name) PGresult *res; int wait_msec; - pgut_command(conn, "SAVEPOINT repack_sp1", 0, NULL); - duration = time(NULL) - start; /* Cancel queries unconditionally, i.e. don't bother waiting @@ -1856,6 +1856,13 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name) } termStringInfo(&sql); + + /* + * Make sure that the current transaction isn't in failed state before + * releasing the savepoint + */ + if (PQtransactionStatus(conn) == PQTRANS_INTRANS) + pgut_command(conn, "RELEASE SAVEPOINT repack_sp1", 0, NULL); pgut_command(conn, "RESET lock_timeout", 0, NULL); return ret; } @@ -1904,9 +1911,10 @@ static bool advisory_lock(PGconn *conn, const char *relid) * conn: connection to use * relid: OID of relation * lock_query: LOCK TABLE ... IN ACCESS EXCLUSIVE query to be executed - * start_xact: whether we will issue a BEGIN ourselves. If not, we will - * use a SAVEPOINT and ROLLBACK TO SAVEPOINT if our query - * times out, to avoid leaving the transaction in error state. + * start_xact: whether we will issue a BEGIN ourselves. Regardless of the + * value we will use a SAVEPOINT and ROLLBACK TO SAVEPOINT if our + * query times out, to avoid leaving the transaction in error + * state. */ static bool lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool start_xact) @@ -1915,6 +1923,10 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta int i; bool ret = true; + if (start_xact) + pgut_command(conn, "BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL); + pgut_command(conn, "SAVEPOINT repack_sp1", 0, NULL); + for (i = 1; ; i++) { time_t duration; @@ -1922,11 +1934,6 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta PGresult *res; int wait_msec; - if (start_xact) - pgut_command(conn, "BEGIN ISOLATION LEVEL READ COMMITTED", 0, NULL); - else - pgut_command(conn, "SAVEPOINT repack_sp1", 0, NULL); - duration = time(NULL) - start; if (duration > wait_timeout) { @@ -1936,10 +1943,7 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta ret = false; /* Before exit the loop reset the transaction */ - if (start_xact) - pgut_rollback(conn); - else - pgut_command(conn, "ROLLBACK TO SAVEPOINT repack_sp1", 0, NULL); + pgut_command(conn, "ROLLBACK TO SAVEPOINT repack_sp1", 0, NULL); break; } else @@ -1982,10 +1986,7 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta { /* retry if lock conflicted */ CLEARPGRES(res); - if (start_xact) - pgut_rollback(conn); - else - pgut_command(conn, "ROLLBACK TO SAVEPOINT repack_sp1", 0, NULL); + pgut_command(conn, "ROLLBACK TO SAVEPOINT repack_sp1", 0, NULL); continue; } else @@ -1998,6 +1999,14 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta } } + /* + * Make sure that the current transaction isn't in failed state before + * releasing the savepoint + */ + if (PQtransactionStatus(conn) == PQTRANS_INTRANS) + pgut_command(conn, "RELEASE SAVEPOINT repack_sp1", 0, NULL); + if (!ret && start_xact) + pgut_rollback(conn); pgut_command(conn, "RESET lock_timeout", 0, NULL); return ret; } diff --git a/regress/expected/nosuper.out b/regress/expected/nosuper.out index 8d0a94ec..588af232 100644 --- a/regress/expected/nosuper.out +++ b/regress/expected/nosuper.out @@ -21,11 +21,7 @@ GRANT USAGE ON SCHEMA repack TO nosuper; -- => ERROR \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check INFO: repacking table "public.tbl_cluster" -ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block -DETAIL: query was: RESET lock_timeout -ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block -DETAIL: query was: RESET lock_timeout -ERROR: permission denied for table tbl_cluster +WARNING: lock_exclusive() failed for public.tbl_cluster ERROR: permission denied for table tbl_cluster REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper; REVOKE USAGE ON SCHEMA repack FROM nosuper; diff --git a/regress/expected/nosuper_1.out b/regress/expected/nosuper_1.out index 3eda71f2..f922549b 100644 --- a/regress/expected/nosuper_1.out +++ b/regress/expected/nosuper_1.out @@ -21,11 +21,7 @@ GRANT USAGE ON SCHEMA repack TO nosuper; -- => ERROR \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check INFO: repacking table "public.tbl_cluster" -ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block -DETAIL: query was: RESET lock_timeout -ERROR: query failed: ERROR: current transaction is aborted, commands ignored until end of transaction block -DETAIL: query was: RESET lock_timeout -ERROR: permission denied for relation tbl_cluster +WARNING: lock_exclusive() failed for public.tbl_cluster ERROR: permission denied for relation tbl_cluster REVOKE ALL ON ALL TABLES IN SCHEMA repack FROM nosuper; REVOKE USAGE ON SCHEMA repack FROM nosuper;