public inbox for [email protected]  

help / color / mirror / Atom feed

From: vignesh C <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Cross-database SERIALIZABLE safe snapshots
Date: Sun, 21 Jan 2024 07:35:53 +0530
Message-ID: <CALDaNm3gpHBkriWU0yHfNHP3_Lv5mOc54FAqXJFNND-qD_c-JQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+hUKGLqUAbpvO-aRznvF2LXnVBuRkYa7NcEG65C7Z0nziWKqw@mail.gmail.com>
	<[email protected]>

On Mon, 10 Jul 2023 at 14:48, Heikki Linnakangas <[email protected]> wrote:
>
> On 09/03/2023 07:34, Thomas Munro wrote:
> > Here is a feature idea that emerged from a pgsql-bugs thread[1] that I
> > am kicking into the next commitfest.  Example:
> >
> > s1: \c db1
> > s1: CREATE TABLE t (i int);
> > s1: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> > s1: INSERT INTO t VALUES (42);
> >
> > s2: \c db2
> > s2: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY DEFERRABLE;
> > s2: SELECT * FROM x;
> >
> > I don't know of any reason why s2 should have to wait, or
> > alternatively, without DEFERRABLE, why it shouldn't immediately drop
> > from SSI to SI (that is, opt out of predicate locking and go faster).
> > This change relies on the fact that PostgreSQL doesn't allow any kind
> > of cross-database access, except for shared catalogs, and all catalogs
> > are already exempt from SSI.  I have updated this new version of the
> > patch to explain that very clearly at the place where that exemption
> > happens, so that future hackers would see that we rely on that fact
> > elsewhere if reconsidering that.
>
> Makes sense.
>
> > @@ -1814,7 +1823,17 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot,
> >                 {
> >                         othersxact = dlist_container(SERIALIZABLEXACT, xactLink, iter.cur);
> >
> > -                       if (!SxactIsCommitted(othersxact)
> > +                       /*
> > +                        * We can't possibly have an unsafe conflict with a transaction in
> > +                        * another database.  The only possible overlap is on shared
> > +                        * catalogs, but we don't support SSI for shared catalogs.  The
> > +                        * invalid database case covers 2PC, because we don't yet record
> > +                        * database OIDs in the 2PC information.  We also filter out doomed
> > +                        * transactions as they can't possibly commit.
> > +                        */
> > +                       if ((othersxact->database == InvalidOid ||
> > +                                othersxact->database == MyDatabaseId)
> > +                               && !SxactIsCommitted(othersxact)
> >                                 && !SxactIsDoomed(othersxact)
> >                                 && !SxactIsReadOnly(othersxact))
> >                         {
>
> Why don't we set the database OID in 2PC transactions? We actually do
> set it correctly - or rather we never clear it - when a transaction is
> prepared. But you set it to invalid when recovering a prepared
> transaction on system startup. So the comment is a bit misleading: the
> optimization doesn't apply to 2PC transactions recovered after restart,
> other 2PC transactions are fine.
>
> I'm sure it's not a big deal in practice, but it's also not hard to fix.
> We do store the database OID in the twophase state. The caller of
> predicatelock_twophase_recover() has it, we just need a little plumbing
> to pass it down.
>
> Attached patches:
>
> 1. Rebased version of your patch, just trivial pgindent conflict fixes
> 2. Some comment typo fixes and improvements
> 3. Set the database ID on recovered 2PC transactions
>
> A test for this would be nice. isolationtester doesn't support
> connecting to different databases, restarting the server to test the 2PC
> recovery, but a TAP test could do it.

@Thomas Munro As this patch is already marked as "Ready for
Committer", do you want to take this patch forward based on Heikki's
suggestions and get it committed?

Regards,
Vignesh

view thread (3+ messages)

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Cross-database SERIALIZABLE safe snapshots
  In-Reply-To: <CALDaNm3gpHBkriWU0yHfNHP3_Lv5mOc54FAqXJFNND-qD_c-JQ@mail.gmail.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