Thread: pg_dump/pg_restore vs default_transaction_read_only under PG 13.2

Dear all,

I am testing the pg_restore of a database with
default_transaction_read_only=on.

The following issue ensues

    sudo -u postgres pg_restore --verbose --create --dbname=template1 --exit-on-error -p 5432
/tmp/gnumed/gm-restore_2021-06-20_18-31-07/backup-gnumed_v22-GNUmed_Team-hermes.dir/
    pg_restore: verbinde mit der Datenbank zur Wiederherstellung
    pg_restore: erstelle DATABASE »gnumed_v22«
    pg_restore: verbinde mit neuer Datenbank »gnumed_v22«
    pg_restore: erstelle DATABASE PROPERTIES »gnumed_v22«
    pg_restore: verbinde mit neuer Datenbank »gnumed_v22«
    pg_restore: erstelle SCHEMA »au«
    pg_restore: in Phase PROCESSING TOC:
    pg_restore: in Inhaltsverzeichniseintrag 7; 2615 16753 SCHEMA au gm-dbo
    pg_restore: Fehler: could not execute query: ERROR:  cannot execute CREATE SCHEMA in a read-only transaction
    Die Anweisung war: CREATE SCHEMA au;

    pg_restore exit code: 1

Producing an SQL file instead of restoring shows which
sequence of events quite logically leads up to this:

    --
    -- PostgreSQL database dump
    --

    -- Dumped from database version 13.2 (Debian 13.2-1)
    -- Dumped by pg_dump version 13.2 (Debian 13.2-1)

    -- Started on 2021-06-20 14:04:46 CEST

    SET statement_timeout = 0;
    SET lock_timeout = 0;
    SET idle_in_transaction_session_timeout = 0;
    SET client_encoding = 'UTF8';
    SET standard_conforming_strings = on;
    SELECT pg_catalog.set_config('search_path', '', false);
    SET check_function_bodies = false;
    SET xmloption = content;
    SET client_min_messages = warning;
    SET row_security = off;

    --
    -- TOC entry 9963 (class 1262 OID 130036)
    -- Name: gnumed_v22; Type: DATABASE; Schema: -; Owner: gm-dbo
    --

    CREATE DATABASE gnumed_v22 WITH TEMPLATE = template0 ENCODING = 'UTF8' LOCALE = 'de_DE.UTF-8';


    ALTER DATABASE gnumed_v22 OWNER TO "gm-dbo";

    \connect gnumed_v22

    SET statement_timeout = 0;
    SET lock_timeout = 0;
    SET idle_in_transaction_session_timeout = 0;
    SET client_encoding = 'UTF8';
    SET standard_conforming_strings = on;
    SELECT pg_catalog.set_config('search_path', '', false);
    SET check_function_bodies = false;
    SET xmloption = content;
    SET client_min_messages = warning;
    SET row_security = off;

    --
    -- TOC entry 9964 (class 0 OID 0)
    -- Name: gnumed_v22; Type: DATABASE PROPERTIES; Schema: -; Owner: gm-dbo
    --

    ALTER DATABASE gnumed_v22 SET lc_messages TO 'C';
    ALTER DATABASE gnumed_v22 SET default_transaction_read_only TO 'on';
    ALTER DATABASE gnumed_v22 SET check_function_bodies TO 'on';
    ALTER DATABASE gnumed_v22 SET ignore_checksum_failure TO 'off';


    \connect gnumed_v22

    SET statement_timeout = 0;
    SET lock_timeout = 0;
    SET idle_in_transaction_session_timeout = 0;
    SET client_encoding = 'UTF8';
    SET standard_conforming_strings = on;
    SELECT pg_catalog.set_config('search_path', '', false);
    SET check_function_bodies = false;
    SET xmloption = content;
    SET client_min_messages = warning;
    SET row_security = off;

    --
    -- TOC entry 7 (class 2615 OID 16753)
    -- Name: au; Type: SCHEMA; Schema: -; Owner: gm-dbo
    --

    CREATE SCHEMA au;

The problem being that pg_restore sets database properties from

    -- TOC entry 9964 (class 0 OID 0)
    -- Name: gnumed_v22; Type: DATABASE PROPERTIES; Schema: -; Owner: gm-dbo

including

    ALTER DATABASE gnumed_v22 SET default_transaction_read_only TO 'on';

(which was, indeed, set to "on" in the dumped database) and
only then attempts to create schema/restore data.

Is this issue handled differently in later versions or should
I be doing something differently during restore ?

It would seem the restore script lacks a

    SET default_transaction_read_only TO 'off';

in the setup section after re-connecting to the DB following
the ALTER DATABASE section ?

Thanks,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

Dear Jain,

> Did I simulate your concern correctly ?

Nearly so, to my understanding. What you did (and thanks for
the followup) was ...

> postgres@db:~/playground/logical_replication$ rm -rf example
> postgres@db:~/playground/logical_replication$ initdb -D example 2>/dev/null
> >/dev/null
> postgres@db:~/playground/logical_replication$ vim example/postgresql.conf
> postgres@db:~/playground/logical_replication$ printf
> "default_transaction_read_only=on\n" >> example/postgresql.conf  #global
> config setting as on

... to tell the server to *treat* all databases in the
cluster as readonly. IOW, an intent rather than a property of
_a_ database (ALTER DATABASE ...).

Consequently, ...

> postgres@db:~/playground/logical_replication$ pg_ctl -D example -l logfile
> start
> waiting for server to start.... done
> server started
> postgres@db:~/playground/logical_replication$ psql
> psql (14beta1)
> Type "help" for help.
>
> postgres=# show default_transaction_read_only;  -- validate it is on
>  default_transaction_read_only
> -------------------------------
>  on
> (1 row)
>
> postgres=# \q
> postgres@db:~/playground/logical_replication$ pg_dumpall -f dump.sql

... when the properties get dumped ...

> postgres@db:~/playground/logical_replication$ grep
> default_transaction_read_only dump.sql -- check what gets dumped
> SET default_transaction_read_only = off;

... the dump does not contain the "on" setting (because, I
assume, it does not dump configuration of the server, but
content and properties of databases).

Still, thanks for thinking along.

Best,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

Karsten Hilbert <Karsten.Hilbert@gmx.net> writes:
> I am testing the pg_restore of a database with
> default_transaction_read_only=on.

Hm.  It's intentional that we reconnect after applying the database
properties, so that they are in effect during the restore.  It seems
likely that failing to do so could result in misbehaviors.

Hence, the only way to make this scenario work would be for the
restore script to explicitly override default_transaction_read_only.
That seems like a darn bad idea, really.  If pg_dump thinks it's
authorized to ignore that, why shouldn't every other application?

Also, doing so would result in ignoring default_transaction_read_only
no matter what the source of that was.  I think it's probably not
hard to construct scenarios where someone would really not be happy
about such behavior.

In short, I'm inclined to say "don't do that".  Maybe it'd be
a better idea to apply default_transaction_read_only to particular
roles (and not run pg_restore under such a role).

            regards, tom lane

Vijaykumar Jain <vijaykumarjain.github@gmail.com> writes:
> i have db example which i want to migrate from one version to another.
> to do that i disable writes on the db by setting the tx flag, so that i can
> be sure it is still serving reads, but no additional data is written.
> i can take a pg_dump, the db is still serving the reads.

What is the point of that?  pg_dump will capture a consistent data
snapshot in any case.  AFAICS, all you accomplish by disabling
writes on the source server is a self-inflicted DOS.

(Also, the fact that default_transaction_read_only is so easy to
override means it's not that useful as a means of disabling writes.)

            regards, tom lane

Am Sun, Jun 20, 2021 at 01:47:47PM -0400 schrieb Tom Lane:

> Hence, the only way to make this scenario work would be for the
> restore script to explicitly override default_transaction_read_only.
[...]
> Also, doing so would result in ignoring default_transaction_read_only
> no matter what the source of that was.  I think it's probably not
> hard to construct scenarios where someone would really not be happy
> about such behavior.

Any chance pg_dump (and/or pg_restore) might gain an option
--ignore-read-only ?   That way, PostgreSQL need not decide
for users.

Thanks,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

Am Sun, Jun 20, 2021 at 09:34:45PM +0200 schrieb Karsten Hilbert:

> Any chance pg_dump (and/or pg_restore) might gain an option
> --ignore-read-only ?   That way, PostgreSQL need not decide
> for users.

Or, options --pre-dump-sql and -post-dump-sql ?

Users could then run setup/teardown SQL for the dump
to end up the way they need it to.

Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

On 6/20/21 12:34 PM, Karsten Hilbert wrote:
> Am Sun, Jun 20, 2021 at 01:47:47PM -0400 schrieb Tom Lane:
> 
>> Hence, the only way to make this scenario work would be for the
>> restore script to explicitly override default_transaction_read_only.
> [...]
>> Also, doing so would result in ignoring default_transaction_read_only
>> no matter what the source of that was.  I think it's probably not
>> hard to construct scenarios where someone would really not be happy
>> about such behavior.
> 
> Any chance pg_dump (and/or pg_restore) might gain an option
> --ignore-read-only ?   That way, PostgreSQL need not decide
> for users.

How about:

1) pg_dump -Fc -d read_only_db -U postgres -f read_only.out

2) In new cluster:
    A) psql -d postgres -U postgres
       CREATE DATABASE read_only;

    B) pg_restore -d read_only -U postgres read_only.out

    C)  psql -d read_only -U postgres
      alter database read_only SET default_transaction_read_only TO 'on';

> 
> Thanks,
> Karsten
> --
> GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B
> 
> 


-- 
Adrian Klaver
adrian.klaver@aklaver.com

Am Sun, Jun 20, 2021 at 01:14:06PM -0700 schrieb Adrian Klaver:

> >Any chance pg_dump (and/or pg_restore) might gain an option
> >--ignore-read-only ?   That way, PostgreSQL need not decide
> >for users.
>
> How about:
>
> 1) pg_dump -Fc -d read_only_db -U postgres -f read_only.out
>
> 2) In new cluster:
>    A) psql -d postgres -U postgres
>       CREATE DATABASE read_only;
>
>    B) pg_restore -d read_only -U postgres read_only.out
>
>    C)  psql -d read_only -U postgres
>      alter database read_only SET default_transaction_read_only TO 'on';

That would work but does not lend itself well to a fully
scripted pg_dump/pg_restore backup "solution". The full
pg_dump command line is this:

    pg_dump --verbose --format=directory --compress=0 --column-inserts --clean --if-exists --serializable-deferrable
"${_PG_HOST_ARG}""${_PG_PORT_ARG}" --username="${GM_DBO}" -f "${BACKUP_DATA_DIR}" "${GM_DATABASE}" 2> /dev/null 

And this is the restore:

    sudo -u postgres pg_restore --verbose --create --dbname=template1 --exit-on-error -p ${GM_PORT} ${BACKUP}.dir/

Thanks,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

On 6/20/21 1:40 PM, Karsten Hilbert wrote:
> Am Sun, Jun 20, 2021 at 01:14:06PM -0700 schrieb Adrian Klaver:
> 
>>> Any chance pg_dump (and/or pg_restore) might gain an option
>>> --ignore-read-only ?   That way, PostgreSQL need not decide
>>> for users.
>>
>> How about:
>>
>> 1) pg_dump -Fc -d read_only_db -U postgres -f read_only.out
>>
>> 2) In new cluster:
>>     A) psql -d postgres -U postgres
>>        CREATE DATABASE read_only;
>>
>>     B) pg_restore -d read_only -U postgres read_only.out
>>
>>     C)  psql -d read_only -U postgres
>>       alter database read_only SET default_transaction_read_only TO 'on';
> 
> That would work but does not lend itself well to a fully
> scripted pg_dump/pg_restore backup "solution". The full
> pg_dump command line is this:
> 
>     pg_dump --verbose --format=directory --compress=0 --column-inserts --clean --if-exists --serializable-deferrable
"${_PG_HOST_ARG}""${_PG_PORT_ARG}" --username="${GM_DBO}" -f "${BACKUP_DATA_DIR}" "${GM_DATABASE}" 2> /dev/null
 
> 

Wouldn't the below just require the following changes?:

> And this is the restore:
> 
    sudo -u postgres psql -d postgres -f create_database.sql
>     sudo -u postgres pg_restore --verbose --create --dbname=template1 --exit-on-error -p ${GM_PORT} ${BACKUP}.dir/
         Remove --create and change dbname to the database name from 
create_database.sql

    sudo -u postgres psql -d <dbname> -f alter_database.sql

At this point version 14 is past the point of adding new features and 15 
is 1+ year out, so there is going to have to be some sort of kludge.

> 
> Thanks,
> Karsten
> --
> GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B
> 


-- 
Adrian Klaver
adrian.klaver@aklaver.com

> >And this is the restore:
> >
>     sudo -u postgres psql -d postgres -f create_database.sql
> >    sudo -u postgres pg_restore --verbose --create --dbname=template1 --exit-on-error -p ${GM_PORT} ${BACKUP}.dir/
>         Remove --create and change dbname to the database name from create_database.sql
>
>     sudo -u postgres psql -d <dbname> -f alter_database.sql
>
> At this point version 14 is past the point of adding new features and 15 is 1+ year out,
> so there is going to have to be some sort of kludge.

I know. I was asking in principle, whether a
felt-to-be-more-proper change in pg_dump/pg_restore for
handling default_transaction_read_only can be hoped to happen
or not (short of submitting a patch).

For the record: The kludge I have implemented now is this

    CMD_PG_RESTORE="pg_restore --create --file=- ${BACKUP}.dir"
    CMD_PSQL="psql --echo-errors --dbname=template1 --port=${GM_PORT} --no-psqlrc"
    CMD_SUDO="sudo --user=postgres"
    {
        ${CMD_PG_RESTORE} | grep --ignore-case --invert-match --regexp="alter database ${TARGET_DB} set
default_transaction_read_onlyto.*on" - | ${CMD_SUDO} ${CMD_PSQL} 
    } &>> "${LOG}"

which I am sure can be improved (such as, I was unable to get
the quoting right for factoring out the grep from the
pipeline :-)

I would be happy to hope that pg_dump might gain knowledge to
skillfully emit

    alter database ... default_transaction_read_only to 'on'

only after having emitted any data-loading statements or else
gain a --do-not-alter-database-to-read-only effectively
omitting said statement thereby allowing for a more elegant
treatment of the issue.

Best,
Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B

Am Fri, Jun 25, 2021 at 11:39:46AM +0200 schrieb Karsten Hilbert:

> I would be happy to hope that pg_dump might
...
> gain a --do-not-alter-database-to-read-only

or even a --(pre|post)-dump-sql=(pre|post).sql allowing for
(here) temporarily altering a role to
default_transaction_read_only=off/on while dumping. If both
pre and post SQL are idempotent they can be applied by
pg_dump regardless of dump failure/success.

But then I can see how that might aggravate remote debugging
by people going "uh, like, we are going to shell out and
temporarily relocate tablespaces to faster storage during
dump" or some such.

Karsten
--
GPG  40BE 5B0E C98E 1713 AFA6  5BC0 3BEA AC80 7D4F C89B