Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set idle_in_transaction_session_timeout to 0 #198

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

NikolayS
Copy link

@NikolayS NikolayS commented Dec 27, 2018

statement_timeout is being set to 0 in session preventing failures
when this parameter is non-zero. This change is to prevent failures due
to idle-in-transaction timeout, which look like this:

FATAL:  terminating connection due to idle-in-transaction timeout
ERROR: query failed: server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
DETAIL: query was: SAVEPOINT repack_sp1

TODO:

  • add version check (idle_in_transaction_session_timeout was added only in Postgres 9.6)
  • fix tests (✋ help is needed)

@NikolayS NikolayS changed the title set idle_in_transaction_session_timeout to 0 [WIP] set idle_in_transaction_session_timeout to 0 Dec 27, 2018
statement_timeout is being set to 0 in session preventing failures
when this parameter is non-zero. This change is to prevent failures due
to idle-in-transaction timeout, which look like this:

```
FATAL:  terminating connection due to idle-in-transaction timeout
ERROR: query failed: server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
DETAIL: query was: SAVEPOINT repack_sp1
```
@NikolayS NikolayS changed the title [WIP] set idle_in_transaction_session_timeout to 0 Set idle_in_transaction_session_timeout to 0 Jan 8, 2019
@NikolayS
Copy link
Author

NikolayS commented Jan 8, 2019

I don't know why tests don't go thru -- please review it in general. Looking forward to feedback.

@dvarrazzo
Copy link
Member

The tests fail with:


+cat regress/regression.diffs
--- /home/travis/build/reorg/pg_repack/regress/expected/nosuper.out	2018-12-28 00:43:59.323529663 +0000
+++ /home/travis/build/reorg/pg_repack/regress/results/nosuper.out	2018-12-28 00:44:29.928251196 +0000
@@ -14,4 +14,6 @@
 -- => ERROR
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
 ERROR: pg_repack failed with error: ERROR:  permission denied for schema repack
+LINE 1: select repack.version(), repack.version_sql(), current_setti...
+               ^
 DROP ROLE IF EXISTS nosuper;

The test script does:

CREATE ROLE nosuper WITH LOGIN;
-- => OK
\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-superuser-check

It seems the error is not related to pg_repack, but to the database configuration for which a the user can't access the repack schema by default. I think after the CREATE ROLE we can add a GRANT USAGE ON SCHEMA repack TO nosuper, in the test and in the matching expected files.

@NikolayS
Copy link
Author

NikolayS commented Jan 10, 2019

@dvarrazzo thank you. However the problem with tests is a bit more complicated – check out this https://travis-ci.org/reorg/pg_repack/jobs/477749374:

ERROR: pg_repack failed with error: You must be a superuser to use pg_repack
 -- => ERROR
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check
-ERROR: pg_repack failed with error: ERROR:  permission denied for schema repack
+ERROR: pg_repack failed with error: ERROR:  permission denied for relation tables
 DROP ROLE IF EXISTS nosuper;
+ERROR:  role "nosuper" cannot be dropped because some objects depend on it
+DETAIL:  privileges for schema repack

Any reasons to use a non-superuser role in tests?

And can you help to fix tests here?

@NikolayS
Copy link
Author

@dvarrazzo kindly pinging you about the questions above

@dvarrazzo
Copy link
Member

I'm sorry I don't have resources now to look into this.

@andreasscherbaum
Copy link
Collaborator

@NikolayS Can you please rebase the PR? This will also trigger the GitHub Actions checks. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants