agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Noah Misch <noah@leadboat.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: robertmhaas@gmail.com
Cc: pgsql-hackers@postgresql.org
Cc: 9erthalion6@gmail.com
Cc: andrew.dunstan@2ndquadrant.com
Cc: hlinnaka@iki.fi
Cc: michael@paquier.xyz
Subject: Re: [HACKERS] WAL logging problem in 9.4.3?
Date: Sat, 18 Jan 2020 19:51:39 -0800
Message-ID: <20200119035139.GA2811524@rfd.leadboat.com> (raw)
In-Reply-To: <20200116.142057.1250623796779593147.horikyota.ntt@gmail.com>
References: <20191226.124639.5401358775142406.horikyota.ntt@gmail.com>
<20200114.193522.177274387863061991.horikyota.ntt@gmail.com>
<20200115.171857.1170166613760719188.horikyota.ntt@gmail.com>
<20200116.142057.1250623796779593147.horikyota.ntt@gmail.com>
On Tue, Jan 14, 2020 at 07:35:22PM +0900, Kyotaro Horiguchi wrote:
> At Thu, 26 Dec 2019 12:46:39 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> > At Wed, 25 Dec 2019 16:15:21 -0800, Noah Misch <noah@leadboat.com> wrote in
> > > === Defect 1: Forgets to skip WAL after SAVEPOINT; DROP TABLE; ROLLBACK TO
> > >
> > > A test in transactions.sql now fails in AssertPendingSyncs_RelationCache(),
> > > when running "make check" under wal_level=minimal. I test this way:
> > >
> > > printf '%s\n%s\n' 'wal_level = minimal' 'max_wal_senders = 0' >$PWD/minimal.conf
> > > make check TEMP_CONFIG=$PWD/minimal.conf
> > >
> > > Self-contained demonstration:
> > > begin;
> > > create table t (c int);
> > > savepoint q; drop table t; rollback to q; -- forgets table is skipping wal
> > > commit; -- assertion failure
>
> This is complex than expected. The DROP TABLE unconditionally removed
> relcache entry. To fix that, I tried to use rd_isinvalid but it failed
> because there's a state that a relcache invalid but the corresponding
> catalog entry is alive.
>
> In the attached patch 0002, I added a boolean in relcache that
> indicates that the relation is already removed in catalog but not
> committed.
This design could work, but some if its properties aren't ideal. For example,
RelationIdGetRelation() can return a !rd_isvalid relation when the relation
has been dropped. What others designs did you consider, if any?
On Thu, Jan 16, 2020 at 02:20:57PM +0900, Kyotaro Horiguchi wrote:
> --- a/src/backend/utils/cache/relcache.c
> +++ b/src/backend/utils/cache/relcache.c
> @@ -3114,8 +3153,10 @@ AtEOXact_cleanup(Relation relation, bool isCommit)
> */
> if (relation->rd_createSubid != InvalidSubTransactionId)
> {
> - if (isCommit)
> - relation->rd_createSubid = InvalidSubTransactionId;
> + relation->rd_createSubid = InvalidSubTransactionId;
> +
> + if (isCommit && !relation->rd_isdropped)
> + {} /* Nothing to do */
What is the purpose of this particular change? This executes at the end of a
top-level transaction. We've already done any necessary syncing, and we're
clearing any flags that caused WAL skipping. I think it's no longer
productive to treat dropped relations differently.
> @@ -3232,6 +3272,19 @@ AtEOSubXact_cleanup(Relation relation, bool isCommit,
> }
> }
>
> + /*
> + * If this relation registered pending sync then dropped, subxact rollback
> + * cancels the uncommitted drop, and commit propagates it to the parent.
> + */
> + if (relation->rd_isdropped)
> + {
> + Assert (!relation->rd_isvalid &&
> + (relation->rd_createSubid != InvalidSubTransactionId ||
> + relation->rd_firstRelfilenodeSubid != InvalidSubTransactionId));
> + if (!isCommit)
> + relation->rd_isdropped = false;
This does the wrong thing when there exists some subtransaction rollback that
does not rollback the DROP:
\pset null 'NULL'
begin;
create extension pg_visibility;
create table droppedtest (c int);
select 'droppedtest'::regclass::oid as oid \gset
savepoint q; drop table droppedtest; release q; -- rd_dropped==true
select * from pg_visibility_map(:oid); -- processes !rd_isvalid rel (not ideal)
savepoint q; select 1; rollback to q; -- rd_dropped==false (wrong)
savepoint q; select 1; rollback to q;
select pg_relation_size(:oid), pg_relation_filepath(:oid),
has_table_privilege(:oid, 'SELECT'); -- all nulls, okay
select * from pg_visibility_map(:oid); -- assertion failure
rollback;
view thread (244+ messages) latest in thread
Message-ID: <20200119035139.GA2811524@rfd.leadboat.com>
Permalink: ../20200119035139.GA2811524@rfd.leadboat.com/
Also on: postgresql.org/message-id/20200119035139.GA2811524@rfd.leadboat.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, horikyota.ntt@gmail.com, robertmhaas@gmail.com, 9erthalion6@gmail.com, andrew.dunstan@2ndquadrant.com, hlinnaka@iki.fi, michael@paquier.xyz
Subject: Re: [HACKERS] WAL logging problem in 9.4.3?
In-Reply-To: <20200119035139.GA2811524@rfd.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox