Thread: Re: [PATCHES] [pgsql-patches] Phantom Command IDs, updated patch
[ time to move this thread to -hackers ]
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> Alvaro Herrera <alvherre@commandprompt.com> writes:
>>> Heikki Linnakangas wrote:
>>>> Tom Lane wrote:
>>>>> BTW, I don't care much for the terminology "phantom cid" ... there's
>>>>> nothing particularly "phantom" about them, seeing they get onto disk.
>>>>> Can anyone think of a better name? The best I can do offhand is
>>>>> "merged cid" or "cid pair", which aren't inspiring.
>>>> MultiCid, like the MultiXacts? Maybe not, they're quite different beasts...
>>> Dual cid? Double cid?
>> "Double cid" doesn't sound too bad. Another thought that just came to
>> mind is "cid interval" or some variant of that.
> I don't like "double ctid" because it is really just one ctid, but
> represents two. I am thinking "packed ctid" is the right wording. It
> doesn't have the same impact as "phantom", but it is probably better.
Packed doesn't seem to have quite the right connotation either --- it
sounds like it means there are two separable fields in the CID value.
Maybe "composite cid"?
Another issue that we need to think about before we go too far with this
is the problem that we punted on before 8.2 release: how to deal with
rolling back an upgrade of a row-level lock from shared to exclusive
within a subtransaction. I'm a bit nervous about committing to merging
cmin and cmax before we have an idea how we're going to solve that ---
it might foreclose a solution. Or maybe we could piggyback on phantom/
composite/whatever CIDs to solve it, which would be great, but let's
try to sketch out a solution now.
regards, tom lane
Tom Lane wrote: > [ time to move this thread to -hackers ] > > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> Alvaro Herrera <alvherre@commandprompt.com> writes: > >>> Heikki Linnakangas wrote: > >>>> Tom Lane wrote: > >>>>> BTW, I don't care much for the terminology "phantom cid" ... there's > >>>>> nothing particularly "phantom" about them, seeing they get onto disk. > >>>>> Can anyone think of a better name? The best I can do offhand is > >>>>> "merged cid" or "cid pair", which aren't inspiring. > > >>>> MultiCid, like the MultiXacts? Maybe not, they're quite different beasts... > > >>> Dual cid? Double cid? > > >> "Double cid" doesn't sound too bad. Another thought that just came to > >> mind is "cid interval" or some variant of that. > > > I don't like "double ctid" because it is really just one ctid, but > > represents two. I am thinking "packed ctid" is the right wording. It > > doesn't have the same impact as "phantom", but it is probably better. > > Packed doesn't seem to have quite the right connotation either --- it > sounds like it means there are two separable fields in the CID value. > > Maybe "composite cid"? At one point I was thinking "combo". but "composite" sounds good. > Another issue that we need to think about before we go too far with this > is the problem that we punted on before 8.2 release: how to deal with > rolling back an upgrade of a row-level lock from shared to exclusive > within a subtransaction. I'm a bit nervous about committing to merging > cmin and cmax before we have an idea how we're going to solve that --- > it might foreclose a solution. Or maybe we could piggyback on phantom/ > composite/whatever CIDs to solve it, which would be great, but let's > try to sketch out a solution now. Good point. Right now we put our new cid on top of the old lock cid, making rollback impossible to the old lock. What if instead of overwriting our old cid with a new one, we create a composite cid, and if we roll back, we look up the composite pair and put the old cid back. It would only work with two cids, but that seems sufficient. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> Packed doesn't seem to have quite the right connotation either --- it
>> sounds like it means there are two separable fields in the CID value.
>>
>> Maybe "composite cid"?
> At one point I was thinking "combo". but "composite" sounds good.
I like "combo" --- nice and short.
>> Another issue that we need to think about before we go too far with this
>> is the problem that we punted on before 8.2 release: how to deal with
>> rolling back an upgrade of a row-level lock from shared to exclusive
>> within a subtransaction. I'm a bit nervous about committing to merging
>> cmin and cmax before we have an idea how we're going to solve that ---
>> it might foreclose a solution. Or maybe we could piggyback on phantom/
>> composite/whatever CIDs to solve it, which would be great, but let's
>> try to sketch out a solution now.
> Good point. Right now we put our new cid on top of the old lock cid,
> making rollback impossible to the old lock. What if instead of
> overwriting our old cid with a new one, we create a composite cid, and
> if we roll back, we look up the composite pair and put the old cid back.
> It would only work with two cids, but that seems sufficient.
Yeah, that's more or less what I was thinking. The problem is that the
composite CID isn't going to be enough info to tell you *where* you have
to put things back. And we don't want to try to remember per-row state
in memory. Is there a way to generalize either the composite CID or the
MultiXact mechanism to support this situation without that?
regards, tom lane
Bruce Momjian wrote: > Tom Lane wrote: > > > > Another issue that we need to think about before we go too far with this > > is the problem that we punted on before 8.2 release: how to deal with > > rolling back an upgrade of a row-level lock from shared to exclusive > > within a subtransaction. I'm a bit nervous about committing to merging > > cmin and cmax before we have an idea how we're going to solve that --- > > it might foreclose a solution. Or maybe we could piggyback on phantom/ > > composite/whatever CIDs to solve it, which would be great, but let's > > try to sketch out a solution now. > > Good point. Right now we put our new cid on top of the old lock cid, > making rollback impossible to the old lock. What if instead of > overwriting our old cid with a new one, we create a composite cid, and > if we roll back, we look up the composite pair and put the old cid back. > It would only work with two cids, but that seems sufficient. This starts to look awfully similar to MultiXactIds. And probably using such a mechanism would allow you to "rollback" any number of row locks: take the current membersoof the "multicid", substract the one that rolled back and use that as new multicid. The main difference is that you'd need to store both the locker Cid and the mode (shared/exclusive). The other difference is that multicids can be stored locally to a backend, no need to have SLRUs etc. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote: > This starts to look awfully similar to MultiXactIds. And probably using > such a mechanism would allow you to "rollback" any number of row locks: > take the current membersoof the "multicid", substract the one that > rolled back and use that as new multicid. The main difference is that > you'd need to store both the locker Cid and the mode (shared/exclusive). Humm, sorry, obviously this makes no sense at all because I mentally mixed the Xid locker and the Cids. > The other difference is that multicids can be stored locally to a > backend, no need to have SLRUs etc. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Tom Lane wrote: > > At one point I was thinking "combo". but "composite" sounds good. > > I like "combo" --- nice and short. > > >> Another issue that we need to think about before we go too far with this > >> is the problem that we punted on before 8.2 release: how to deal with > >> rolling back an upgrade of a row-level lock from shared to exclusive > >> within a subtransaction. I'm a bit nervous about committing to merging > >> cmin and cmax before we have an idea how we're going to solve that --- > >> it might foreclose a solution. Or maybe we could piggyback on phantom/ > >> composite/whatever CIDs to solve it, which would be great, but let's > >> try to sketch out a solution now. > > > Good point. Right now we put our new cid on top of the old lock cid, > > making rollback impossible to the old lock. What if instead of > > overwriting our old cid with a new one, we create a composite cid, and > > if we roll back, we look up the composite pair and put the old cid back. > > It would only work with two cids, but that seems sufficient. > > Yeah, that's more or less what I was thinking. The problem is that the > composite CID isn't going to be enough info to tell you *where* you have > to put things back. And we don't want to try to remember per-row state > in memory. Is there a way to generalize either the composite CID or the > MultiXact mechanism to support this situation without that? Uh, well, hmmm. The way combo cid is supposed to work is that you are deleting a row created in your same transaction by a previous command id, so you look in the combo cid array to see if a match for that pair exists --- if not, you create a new entry and put the two cids on it. So, with the combo lock cid, you do the same process, and lookups of who holds the lock looks at the cid combo, and if the second subtransaction was aborted, the first one is the lock holder. If you again lock the row, you create a new combo cid and use the original cid there because the second cid was aborted. I don't see how any of this is per-row for locks anymore than it is per-row for insert/delete. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Humm, sorry, obviously this makes no sense at all because I mentally
> mixed the Xid locker and the Cids.
After thinking a bit, I have a sketch of a solution.
Assume that we extend the MultiXact infrastructure so that it can track
whether each member of a MultiXact holds shared or exclusive lock.
(There are a couple ways you could do that --- add a parallel bit-array,
or separate the members into two groups. Details not important for now.)
The only way you could have both shared- and exclusive-lock members is
if they are subtransactions of the same backend, but that fact isn't
real relevant to the MultiXact code.
Then extend MultiXactIdWait() so that you can tell it to wait for all
members to die, or just the exclusive members.
Then the representation of the problem situation would be that a locked
tuple would have as XMAX a MultiXact containing the upper XID as shared
locker and the subtransaction as exclusive locker. Onlookers could wait
for one or both to die as appropriate depending on what kind of lock
they needed to get. HEAP_XMAX_EXCL_LOCK would have to be a hint rather
than the truth, ie, once all the exclusive-lock members of the MultiXact
are dead it's really only a shared lock, but I don't see that this poses
any real difficulty.
I don't particularly want to go implement this now; I just want a
proof-of-concept sketch proving that we don't need separate cmin and
cmax to support this.
As for what I think we *should* do near-term, I'm pretty strongly
tempted to suggest that we just throw an error if a subtransaction tries
to upgrade an upper transaction's shared lock to exclusive. When and if
we get a ton of complaints about that, it'd be time to put forth effort
to fix it. I suspect the situation doesn't really arise much in
practice, else we'd have heard complaints from the field about the fact
that the shared lock can become lost.
Comments?
regards, tom lane
Bruce Momjian <bruce@momjian.us> writes:
> The way combo cid is supposed to work is that you are deleting a row
> created in your same transaction by a previous command id, so you look
> in the combo cid array to see if a match for that pair exists --- if
> not, you create a new entry and put the two cids on it.
> So, with the combo lock cid, you do the same process, and lookups of who
> holds the lock looks at the cid combo, and if the second subtransaction
> was aborted, the first one is the lock holder. If you again lock the
> row, you create a new combo cid and use the original cid there because
> the second cid was aborted.
No, because no process other than the originator can see the combo-cid
data structure, and for locking situations you really need other
backends to be able to know whether the tuple is locked and how.
But I think my proposal of extending MultiXact would fix it; please look
at that.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > The way combo cid is supposed to work is that you are deleting a row > > created in your same transaction by a previous command id, so you look > > in the combo cid array to see if a match for that pair exists --- if > > not, you create a new entry and put the two cids on it. > > > So, with the combo lock cid, you do the same process, and lookups of who > > holds the lock looks at the cid combo, and if the second subtransaction > > was aborted, the first one is the lock holder. If you again lock the > > row, you create a new combo cid and use the original cid there because > > the second cid was aborted. > > No, because no process other than the originator can see the combo-cid > data structure, and for locking situations you really need other > backends to be able to know whether the tuple is locked and how. Oh, OK, I forgot pg_subtrans is visible to all backends. > But I think my proposal of extending MultiXact would fix it; please look > at that. Sounds good. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Thu, 2007-02-08 at 15:32 -0500, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> Packed doesn't seem to have quite the right connotation either --- it > >> sounds like it means there are two separable fields in the CID value. > >> > >> Maybe "composite cid"? > > > At one point I was thinking "combo". but "composite" sounds good. Combo is OK, because it's a *combination* of two CommandIds. That means they are ComboCommandIds or CCIs. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Simon Riggs wrote: > On Thu, 2007-02-08 at 15:32 -0500, Tom Lane wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > > Tom Lane wrote: > > >> Packed doesn't seem to have quite the right connotation either --- it > > >> sounds like it means there are two separable fields in the CID value. > > >> > > >> Maybe "composite cid"? > > > > > At one point I was thinking "combo". but "composite" sounds good. > > Combo is OK, because it's a *combination* of two CommandIds. > > That means they are ComboCommandIds or CCIs. CCI is CommandCounterIncrement to me, so let's not use that abbreviation. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
On Fri, 2007-02-09 at 00:08 -0300, Alvaro Herrera wrote: > Simon Riggs wrote: > > On Thu, 2007-02-08 at 15:32 -0500, Tom Lane wrote: > > > Bruce Momjian <bruce@momjian.us> writes: > > > > Tom Lane wrote: > > > >> Packed doesn't seem to have quite the right connotation either --- it > > > >> sounds like it means there are two separable fields in the CID value. > > > >> > > > >> Maybe "composite cid"? > > > > > > > At one point I was thinking "combo". but "composite" sounds good. > > > > Combo is OK, because it's a *combination* of two CommandIds. > > > > That means they are ComboCommandIds or CCIs. > > CCI is CommandCounterIncrement to me, so let's not use that > abbreviation. True; given the similar context that would be a mistake. Just ComboIds then? I was worried that was cid also. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Simon Riggs wrote:
>> Combo is OK, because it's a *combination* of two CommandIds.
>>
>> That means they are ComboCommandIds or CCIs.
> CCI is CommandCounterIncrement to me, so let's not use that
> abbreviation.
Agreed. I looked for a bit at adding a separate typedef ComboCommandId
to be used where a cid was definitely a combo cid, but it didn't seem to
be very useful to do that...
I'm testing the patch currently. I was a bit surprised to find the
without_oid test failing, but it makes sense because I'm using a
MAXALIGN=8 machine. I suppose Heikki tested on MAXALIGN=4.
regards, tom lane
On Thu, 2007-02-08 at 22:15 -0500, Tom Lane wrote: > I'm testing the patch currently. I was a bit surprised to find the > without_oid test failing, but it makes sense because I'm using a > MAXALIGN=8 machine. I suppose Heikki tested on MAXALIGN=4. That's definitely strange. The patch has been performance tested on a MAXALIGN=8 system on tables without OIDs on, but I'm not sure we ran the make check itself on that server. Will do that in future. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Tom Lane wrote: > I'm testing the patch currently. I was a bit surprised to find the > without_oid test failing, but it makes sense because I'm using a > MAXALIGN=8 machine. I suppose Heikki tested on MAXALIGN=4. That's right. Thanks for the review! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] [pgsql-patches] Phantom Command IDs, updated patch
From
"Zeugswetter Andreas ADI SD"
Date:
> As for what I think we *should* do near-term, I'm pretty strongly > tempted to suggest that we just throw an error if a subtransaction tries > to upgrade an upper transaction's shared lock to exclusive. So when a RI check locks a parent, you would not be able to update the parent in a later subtrans. I can imagine, that the error would be a problem in a select for update loop, because there you usually want to update the row. Andreas
"Zeugswetter Andreas ADI SD" <ZeugswetterA@spardat.at> writes:
> So when a RI check locks a parent, you would not be able to update the
> parent in a later subtrans. I can imagine, that the error would be a
> problem in a select for update loop, because there you usually want to
> update the row.
No, it would not, because select for update would acquire exclusive lock
in the first place.
regards, tom lane