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

[bitnami/postgresql] Introduces environment variable POSTGRESQL_DEFAULT_TRANSACTION_ISOLATION to change the global default transaction isolation level #47819

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

cowclaw
Copy link
Contributor

@cowclaw cowclaw commented Sep 8, 2023

Description of the change

The change introduces a new environment variable POSTGRESQL_DEFAULT_TRANSACTION_ISOLATION to the bitnami postgres images. The environment variable can be used to change the global default for the transaction isolation level in postgresql.

The change fixes #25013

It's currently not possible to change the default transaction isolation level using the POSTGRESQL_EXTRA_FLAGS if the desired isolation level's name contains a space.

if [[ -n "${POSTGRESQL_EXTRA_FLAGS:-}" ]]; then
read -r -a extra_flags <<< "$POSTGRESQL_EXTRA_FLAGS"
flags+=("${extra_flags[@]}")
fi

If we'd omit the -r flag given to read in it would work with escaping the space in "repeatable read" with a backslash (repeatable\ read). However that this is not possible to preserve backwards compatibility since it could break existing solutions.

Benefits

It preserves backwards compatibility but still offers a possibility to change the global default transaction isolation level.

Possible drawbacks

It does not use the existing environment variable POSTGRESQL_EXTRA_FLAGS.

Applicable issues

--

Additional information

I verified the change is correct using the below method.

For each folder in

  • bitnami/postgresql/11/debian-11/
  • bitnami/postgresql/12/debian-11/
  • bitnami/postgresql/13/debian-11/
  • bitnami/postgresql/14/debian-11/
  • bitnami/postgresql/15/debian-11/

do

docker build -t postgres_tmp .
docker run -eALLOW_EMPTY_PASSWORD=yes --rm --name postgresql postgres_tmp
echo "SELECT current_setting('transaction_isolation');" | docker exec -i postgresql psql postgres postgres 

 current_setting 
-----------------
 read committed
(1 row)
docker run -ePOSTGRESQL_DEFAULT_TRANSACTION_ISOLATION="repeatable read" -eALLOW_EMPTY_PASSWORD=yes --rm --name postgresql postgres_tmp 
echo "SELECT current_setting('transaction_isolation');" | docker exec -i postgresql psql postgres postgres 

 current_setting 
-----------------
 repeatable read
(1 row)

@github-actions github-actions bot added the triage Triage is needed label Sep 8, 2023
@cowclaw cowclaw force-pushed the main branch 3 times, most recently from f197394 to 9542b3f Compare September 8, 2023 13:30
@javsalgar javsalgar changed the title Introduces environment variable POSTGRESQL_DEFAULT_TRANSACTION_ISOLATION to change the global default transaction isolation level [bitnami/postgresql] Introduces environment variable POSTGRESQL_DEFAULT_TRANSACTION_ISOLATION to change the global default transaction isolation level Sep 11, 2023
@javsalgar javsalgar added the verify Execute verification workflow for these changes label Sep 11, 2023
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Sep 11, 2023
@bitnami-bot bitnami-bot removed the request for review from javsalgar September 11, 2023 08:01
Copy link
Contributor

@rafariossaa rafariossaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
Thanks for adding this.
This new environment variable should be also added to postgresql-env.sh

@cowclaw
Copy link
Contributor Author

cowclaw commented Sep 14, 2023

Hi @rafariossaa

Thanks for the review.

I updated postgresql-env.sh in df9866a

Copy link
Contributor

@rafariossaa rafariossaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@rafariossaa rafariossaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rafariossaa rafariossaa merged commit 3e4b614 into bitnami:main Sep 18, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
postgresql solved verify Execute verification workflow for these changes
Projects
None yet
3 participants