Skip to content
New issue

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

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

Already on GitHub? # 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.

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

Successfully merging this pull request may close these issues.

3 participants