Re: [HACKERS] SERIALIZABLE with parallel query - Mailing list pgsql-hackers
From | Thomas Munro |
---|---|
Subject | Re: [HACKERS] SERIALIZABLE with parallel query |
Date | |
Msg-id | CAEepm=09Mht4gSY==DEALCcx1TwJ=dQJ4gj6xmtQNEHVnWNmxw@mail.gmail.com Whole thread Raw |
In response to | Re: [HACKERS] SERIALIZABLE with parallel query (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: [HACKERS] SERIALIZABLE with parallel query
(Thomas Munro <thomas.munro@enterprisedb.com>)
Re: [HACKERS] SERIALIZABLE with parallel query (Robert Haas <robertmhaas@gmail.com>) |
List | pgsql-hackers |
On Fri, Jan 26, 2018 at 4:24 AM, Robert Haas <robertmhaas@gmail.com> wrote: > I took a look at this today and thought it might be OK to commit, Thank you for looking at this! > modulo a few minor issues: (1) you didn't document the new tranche and Fixed. > (2) I prefer to avoid if (blah) { Assert(thing) } in favor of > Assert(!blah || thing). Done. > But then I got a little bit concerned about ReleasePredicateLocks(). > Suppose that in the middle of a read-only transaction, > SXACT_FLAG_RO_SAFE becomes true. The next call to > SerializationNeededForRead in each process will call > ReleasePredicateLocks. In the workers, this won't do anything, so > we'll just keep coming back there. But in the leader, we'll go ahead > do all that stuff, including MySerializableXact = > InvalidSerializableXact. But in the workers, it's still set. Maybe > that's OK, but I'm not sure that it's OK... Ouch. Yeah. It's not OK. If the leader gives up its SERIALIZABLEXACT object early due to that safe-read-only optimisation, the workers are left with a dangling pointer to a SERIALIZABLEXACT object that has been pushed onto FinishedSerializableTransactions. From there it will move to PredXact->availableTransactions and might be handed out to some other transaction, so it is not safe to retain a pointer to that object. The best solution I have come up with so far is to add a reference count to SERIALIZABLEXACT. I toyed with putting the refcount into the DSM instead, but then I ran into problems making that work when you have a query with multiple Gather nodes. Since the refcount is in SERIALIZABLEXACT I also had to add a generation counter so that I could detect the case where you try to attach too late (the leader has already errored out, the refcount has reached 0 and the SERIALIZABLEXACT object has been recycled). The attached is a draft patch only, needing some testing and polish. Brickbats, better ideas? FWIW I also considered a couple of other ideas: (1) Keeping the object alive on the FinishedSerializableTransactions list until the leader's transaction is finished seems broken because we need to be able to spill that list to the SLRU at any time, and if we somehow made them sticky we could run out of memory. (2) Anything involving the leader having sole control of the object lifetime seems problematic... well, it might work if you disabled the SXACT_FLAG_RO_SAFE optimisation so that ReleasePredicateLocks() always happens after all workers have finished, but that seems like cheating. PS I noticed that for BecomeLockGroupMember() we say "If we can't join the lock group, the leader has gone away, so just exit quietly" but for various other similar things we spew errors (most commonly seen one being "ERROR: could not map dynamic shared memory segment"). Intentional? -- Thomas Munro http://www.enterprisedb.com
Attachment
pgsql-hackers by date: