Thread: SSI freezing bug
Hi, Prompted by Andres Freund's comments on my Freezing without Write I/O patch, I realized that there's there's an existing bug in the way predicate locking handles freezing (or rather, it doesn't handle it). When a tuple is predicate-locked, the key of the lock is ctid+xmin. However, when a tuple is frozen, its xmin is changed to FrozenXid. That effectively invalidates any predicate lock on the tuple, as checking for a lock on the same tuple later won't find it as the xmin is different. Attached is an isolationtester spec to demonstrate this. - Heikki
Attachment
Hi, On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: > When a tuple is predicate-locked, the key of the lock is ctid+xmin. However, > when a tuple is frozen, its xmin is changed to FrozenXid. That effectively > invalidates any predicate lock on the tuple, as checking for a lock on the > same tuple later won't find it as the xmin is different. > > Attached is an isolationtester spec to demonstrate this. Do you have any idea to fix that besides keeping the xmin horizon below the lowest of the xids that are predicate locked? Which seems nasty to compute and is probably not trivial to fit into the procarray.c machinery? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 2013-09-20 13:53:04 +0200, Andres Freund wrote: > Hi, > > > On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: > > When a tuple is predicate-locked, the key of the lock is ctid+xmin. However, > > when a tuple is frozen, its xmin is changed to FrozenXid. That effectively > > invalidates any predicate lock on the tuple, as checking for a lock on the > > same tuple later won't find it as the xmin is different. > > > > Attached is an isolationtester spec to demonstrate this. > > Do you have any idea to fix that besides keeping the xmin horizon below the > lowest of the xids that are predicate locked? Which seems nasty to > compute and is probably not trivial to fit into the procarray.c > machinery? A better solution probably is to promote tuple-level locks if they exist to a relation level one upon freezing I guess? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> wrote: >> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>> When a tuple is predicate-locked, the key of the lock is ctid+xmin. >>> However, when a tuple is frozen, its xmin is changed to FrozenXid. That >>> effectively >>> invalidates any predicate lock on the tuple, as checking for a lock on >>> the >>> same tuple later won't find it as the xmin is different. >>> >>> Attached is an isolationtester spec to demonstrate this. >> >> Do you have any idea to fix that besides keeping the xmin horizon below the >> lowest of the xids that are predicate locked? Which seems nasty to >> compute and is probably not trivial to fit into the procarray.c >> machinery? > > A better solution probably is to promote tuple-level locks if they exist > to a relation level one upon freezing I guess? That would work. A couple other ideas would be to use the oldest serializable xmin which is being calculated in predicate.c to either prevent freezing of newer transaction or to summarize predicate locks (using the existing SLRU-based summarization functions) which use xmin values for xids which are being frozen. The transactions must already be committed, and so are eligible for summarization. I'm not sure which is best. Will review, probably this weekend. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Kevin Grittner <kgrittn@ymail.com> wrote: >Andres Freund <andres@2ndquadrant.com> wrote: >>> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>>> When a tuple is predicate-locked, the key of the lock is ctid+xmin. >>>> However, when a tuple is frozen, its xmin is changed to FrozenXid. >That >>>> effectively >>>> invalidates any predicate lock on the tuple, as checking for a lock >on >>>> the >>>> same tuple later won't find it as the xmin is different. >>>> >>>> Attached is an isolationtester spec to demonstrate this. >>> >>> Do you have any idea to fix that besides keeping the xmin horizon >below the >>> lowest of the xids that are predicate locked? Which seems nasty to >>> compute and is probably not trivial to fit into the procarray.c >>> machinery? >> >> A better solution probably is to promote tuple-level locks if they >exist >> to a relation level one upon freezing I guess? > >That would work. A couple other ideas would be to use the oldest >serializable xmin which is being calculated in predicate.c to >either prevent freezing of newer transaction or to summarize >predicate locks (using the existing SLRU-based summarization >functions) which use xmin values for xids which are being frozen. >The transactions must already be committed, and so are eligible for >summarization. What's the point of using xmin as part of the lock key in the first place, wouldn't ctid alone suffice? If the tuple wasvisible to the reading transaction, it cannot be vacuumed away until the transaction commits. No other tuple can appearwith the same ctid. - Heikki
Heikki Linnakangas <hlinnakangas@vmware.com> schrieb: >Kevin Grittner <kgrittn@ymail.com> wrote: >>Andres Freund <andres@2ndquadrant.com> wrote: >>>> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>>>> When a tuple is predicate-locked, the key of the lock is >ctid+xmin. >>>>> However, when a tuple is frozen, its xmin is changed to FrozenXid. >>That >>>>> effectively >>>>> invalidates any predicate lock on the tuple, as checking for a >lock >>on >>>>> the >>>>> same tuple later won't find it as the xmin is different. >>>>> >>>>> Attached is an isolationtester spec to demonstrate this. >>>> >>>> Do you have any idea to fix that besides keeping the xmin horizon >>below the >>>> lowest of the xids that are predicate locked? Which seems nasty to >>>> compute and is probably not trivial to fit into the procarray.c >>>> machinery? >>> >>> A better solution probably is to promote tuple-level locks if they >>exist >>> to a relation level one upon freezing I guess? >> >>That would work. A couple other ideas would be to use the oldest >>serializable xmin which is being calculated in predicate.c to >>either prevent freezing of newer transaction or to summarize >>predicate locks (using the existing SLRU-based summarization >>functions) which use xmin values for xids which are being frozen. >>The transactions must already be committed, and so are eligible for >>summarization. > >What's the point of using xmin as part of the lock key in the first >place, wouldn't ctid alone suffice? If the tuple was visible to the >reading transaction, it cannot be vacuumed away until the transaction >commits. No other tuple can appear with the same ctid. SSI locks can live longer than one TX/snapshot. Andres -- Please excuse brevity and formatting - I am writing this on my mobile phone. Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 09/21/2013 10:46 PM, Andres Freund wrote: > > Heikki Linnakangas <hlinnakangas@vmware.com> schrieb: >> Kevin Grittner <kgrittn@ymail.com> wrote: >>> Andres Freund <andres@2ndquadrant.com> wrote: >>>>> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>>>>> When a tuple is predicate-locked, the key of the lock is >> ctid+xmin. >>>>>> However, when a tuple is frozen, its xmin is changed to FrozenXid. >>> That >>>>>> effectively >>>>>> invalidates any predicate lock on the tuple, as checking for a >> lock >>> on >>>>>> the >>>>>> same tuple later won't find it as the xmin is different. >>>>>> >>>>>> Attached is an isolationtester spec to demonstrate this. >>>>> Do you have any idea to fix that besides keeping the xmin horizon >>> below the >>>>> lowest of the xids that are predicate locked? Which seems nasty to >>>>> compute and is probably not trivial to fit into the procarray.c >>>>> machinery? >>>> A better solution probably is to promote tuple-level locks if they >>> exist >>>> to a relation level one upon freezing I guess? >>> That would work. A couple other ideas would be to use the oldest >>> serializable xmin which is being calculated in predicate.c to >>> either prevent freezing of newer transaction or to summarize >>> predicate locks (using the existing SLRU-based summarization >>> functions) which use xmin values for xids which are being frozen. >>> The transactions must already be committed, and so are eligible for >>> summarization. >> What's the point of using xmin as part of the lock key in the first >> place, wouldn't ctid alone suffice? If the tuple was visible to the >> reading transaction, it cannot be vacuumed away until the transaction >> commits. No other tuple can appear with the same ctid. > SSI locks can live longer than one TX/snapshot. But the only way that ctid can change without xmin changing is when you update the tuple in the same TX that created it. Could it be the case here or can we safely exclude this ? -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
On 09/20/2013 12:55 PM, Heikki Linnakangas wrote: > Hi, > > Prompted by Andres Freund's comments on my Freezing without Write I/O > patch, I realized that there's there's an existing bug in the way > predicate locking handles freezing (or rather, it doesn't handle it). > > When a tuple is predicate-locked, the key of the lock is ctid+xmin. > However, when a tuple is frozen, its xmin is changed to FrozenXid. > That effectively invalidates any predicate lock on the tuple, as > checking for a lock on the same tuple later won't find it as the xmin > is different. > > Attached is an isolationtester spec to demonstrate this. The case is even fishier than that. That is, you can get bad behaviour on at least v9.2.4 even without VACUUM FREEZE. You just need to run permutation "r1" "r2" "w1" "w2" "c1" "c2" twice in a row. the first time it does get serialization error at "c2" but the 2nd time both "c1" and "c2" complete successfully Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
On 22.09.2013 00:12, Hannu Krosing wrote: > On 09/21/2013 10:46 PM, Andres Freund wrote: >> >> Heikki Linnakangas<hlinnakangas@vmware.com> schrieb: >>> Kevin Grittner<kgrittn@ymail.com> wrote: >>>> Andres Freund<andres@2ndquadrant.com> wrote: >>>>>> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>>>>>> When a tuple is predicate-locked, the key of the lock is >>> ctid+xmin. >>>>>>> However, when a tuple is frozen, its xmin is changed to FrozenXid. >>>> That >>>>>>> effectively >>>>>>> invalidates any predicate lock on the tuple, as checking for a >>> lock >>>> on >>>>>>> the >>>>>>> same tuple later won't find it as the xmin is different. >>>>>>> >>>>>>> Attached is an isolationtester spec to demonstrate this. >>>>>> Do you have any idea to fix that besides keeping the xmin horizon >>>> below the >>>>>> lowest of the xids that are predicate locked? Which seems nasty to >>>>>> compute and is probably not trivial to fit into the procarray.c >>>>>> machinery? >>>>> A better solution probably is to promote tuple-level locks if they >>>> exist >>>>> to a relation level one upon freezing I guess? >>>> That would work. A couple other ideas would be to use the oldest >>>> serializable xmin which is being calculated in predicate.c to >>>> either prevent freezing of newer transaction or to summarize >>>> predicate locks (using the existing SLRU-based summarization >>>> functions) which use xmin values for xids which are being frozen. >>>> The transactions must already be committed, and so are eligible for >>>> summarization. >>> What's the point of using xmin as part of the lock key in the first >>> place, wouldn't ctid alone suffice? If the tuple was visible to the >>> reading transaction, it cannot be vacuumed away until the transaction >>> commits. No other tuple can appear with the same ctid. >> SSI locks can live longer than one TX/snapshot. > But the only way that ctid can change without xmin changing is when > you update the tuple in the same TX that created it. I don't think that's relevant. We're not talking about the "ctid" field of a tuple, ie. HeapTupleHeader.t_ctid. We're talking about its physical location, ie. HeapTuple->t_self. - Heikki
On 21.09.2013 23:46, Andres Freund wrote: > > > Heikki Linnakangas<hlinnakangas@vmware.com> schrieb: >> Kevin Grittner<kgrittn@ymail.com> wrote: >>> Andres Freund<andres@2ndquadrant.com> wrote: >>>>> On 2013-09-20 13:55:36 +0300, Heikki Linnakangas wrote: >>>>>> When a tuple is predicate-locked, the key of the lock is >> ctid+xmin. >>>>>> However, when a tuple is frozen, its xmin is changed to FrozenXid. >>> That >>>>>> effectively >>>>>> invalidates any predicate lock on the tuple, as checking for a >> lock >>> on >>>>>> the >>>>>> same tuple later won't find it as the xmin is different. >>>>>> >>>>>> Attached is an isolationtester spec to demonstrate this. >>>>> >>>>> Do you have any idea to fix that besides keeping the xmin horizon >>> below the >>>>> lowest of the xids that are predicate locked? Which seems nasty to >>>>> compute and is probably not trivial to fit into the procarray.c >>>>> machinery? >>>> >>>> A better solution probably is to promote tuple-level locks if they >>> exist >>>> to a relation level one upon freezing I guess? >>> >>> That would work. A couple other ideas would be to use the oldest >>> serializable xmin which is being calculated in predicate.c to >>> either prevent freezing of newer transaction or to summarize >>> predicate locks (using the existing SLRU-based summarization >>> functions) which use xmin values for xids which are being frozen. >>> The transactions must already be committed, and so are eligible for >>> summarization. >> >> What's the point of using xmin as part of the lock key in the first >> place, wouldn't ctid alone suffice? If the tuple was visible to the >> reading transaction, it cannot be vacuumed away until the transaction >> commits. No other tuple can appear with the same ctid. > > SSI locks can live longer than one TX/snapshot. Hmm, true. Another idea: we could vacuum away predicate locks, when the corresponding tuples are vacuumed from the heap. Before the 2nd phase of vacuum, before removing the dead tuples, we could scan the predicate lock hash and remove any locks belonging to the tuples we're about to remove. And not include xmin in the lock key. - Heikki
On 23.09.2013 01:07, Hannu Krosing wrote: > On 09/20/2013 12:55 PM, Heikki Linnakangas wrote: >> Hi, >> >> Prompted by Andres Freund's comments on my Freezing without Write I/O >> patch, I realized that there's there's an existing bug in the way >> predicate locking handles freezing (or rather, it doesn't handle it). >> >> When a tuple is predicate-locked, the key of the lock is ctid+xmin. >> However, when a tuple is frozen, its xmin is changed to FrozenXid. >> That effectively invalidates any predicate lock on the tuple, as >> checking for a lock on the same tuple later won't find it as the xmin >> is different. >> >> Attached is an isolationtester spec to demonstrate this. > The case is even fishier than that. > > That is, you can get bad behaviour on at least v9.2.4 even without > VACUUM FREEZE. > > You just need to run > > permutation "r1" "r2" "w1" "w2" "c1" "c2" > > twice in a row. > > the first time it does get serialization error at "c2" > but the 2nd time both "c1" and "c2" complete successfully Oh, interesting. I did some debugging on this: there are actually *two* bugs, either one of which alone is enough to cause this on its own: 1. in heap_hot_search_buffer(), the PredicateLockTuple() call is passed wrong offset number. heapTuple->t_self is set to the tid of the first tuple in the chain that's visited, not the one actually being read. 2. CheckForSerializableConflictIn() uses the tuple's t_ctid field instead of t_self to check for exiting predicate locks on the tuple. If the tuple was updated, but the updater rolled back, t_ctid points to the aborted dead tuple. After fixing both of those bugs, running the test case twice in a row works, ie. causes a conflict and a rollback both times. Anyone see a problem with this? That still leaves the original problem I spotted, with freezing; that's yet another unrelated bug. - Heikki
Attachment
Andres Freund <andres@2ndquadrant.com> wrote: > A better solution probably is to promote tuple-level locks if > they exist to a relation level one upon freezing I guess? It would be sufficient to promote the tuple lock to a page lock. It would be pretty easy to add a function to predicate.c which would accept a Relation and HeapTuple, check for a predicate lock for the tuple, and add a page lock if found (which will automatically clear the tuple lock). This new function would be called when a tuple was chosen for freezing. Since freezing always causes WAL-logging and disk I/O, the cost of a couple hash table operations should not be noticeable. This seems like a bug fix which should be back-patched to 9.1, yes? -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2013-10-01 07:41:46 -0700, Kevin Grittner wrote: > Andres Freund <andres@2ndquadrant.com> wrote: > > > A better solution probably is to promote tuple-level locks if > > they exist to a relation level one upon freezing I guess? > > It would be sufficient to promote the tuple lock to a page lock. > It would be pretty easy to add a function to predicate.c which > would accept a Relation and HeapTuple, check for a predicate lock > for the tuple, and add a page lock if found (which will > automatically clear the tuple lock). This new function would be > called when a tuple was chosen for freezing. Since freezing always > causes WAL-logging and disk I/O, the cost of a couple hash table > operations should not be noticeable. Yea, not sure why I was thinking of table level locks. > This seems like a bug fix which should be back-patched to 9.1, yes? Yes. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> wrote: > On 2013-10-01 07:41:46 -0700, Kevin Grittner wrote: >> Andres Freund <andres@2ndquadrant.com> wrote: >> >>> A better solution probably is to promote tuple-level locks if >>> they exist to a relation level one upon freezing I guess? >> >> It would be sufficient to promote the tuple lock to a page lock. >> It would be pretty easy to add a function to predicate.c which >> would accept a Relation and HeapTuple, check for a predicate lock >> for the tuple, and add a page lock if found (which will >> automatically clear the tuple lock). This new function would be >> called when a tuple was chosen for freezing. Since freezing always >> causes WAL-logging and disk I/O, the cost of a couple hash table >> operations should not be noticeable. > > Yea, not sure why I was thinking of table level locks. > >> This seems like a bug fix which should be back-patched to 9.1, yes? > > Yes. Patch attached, including new isolation test based on Heikki's example. This patch does change the signature of heap_freeze_tuple(). If anyone thinks there is risk that external code may be calling this, I could keep the old function with its old behavior (including the bug) and add a new function name with the new parameters added -- the old function could call the new one with NULL for the last two parameters. I'm not sure whether that is better than breaking a compile of code which uses the old signature, which would force a choice about what to do. Will give it a couple days for feedback before pushing. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On 03.10.2013 01:05, Kevin Grittner wrote: > Andres Freund<andres@2ndquadrant.com> wrote: >> On 2013-10-01 07:41:46 -0700, Kevin Grittner wrote: >>> Andres Freund<andres@2ndquadrant.com> wrote: >>> >>>> A better solution probably is to promote tuple-level locks if >>>> they exist to a relation level one upon freezing I guess? >>> >>> It would be sufficient to promote the tuple lock to a page lock. >>> It would be pretty easy to add a function to predicate.c which >>> would accept a Relation and HeapTuple, check for a predicate lock >>> for the tuple, and add a page lock if found (which will >>> automatically clear the tuple lock). This new function would be >>> called when a tuple was chosen for freezing. Since freezing always >>> causes WAL-logging and disk I/O, the cost of a couple hash table >>> operations should not be noticeable. >> >> Yea, not sure why I was thinking of table level locks. >> >>> This seems like a bug fix which should be back-patched to 9.1, yes? >> >> Yes. > > Patch attached, including new isolation test based on Heikki's > example. This patch does change the signature of > heap_freeze_tuple(). If anyone thinks there is risk that external > code may be calling this, I could keep the old function with its > old behavior (including the bug) and add a new function name with > the new parameters added -- the old function could call the new one > with NULL for the last two parameters. I'm not sure whether that > is better than breaking a compile of code which uses the old > signature, which would force a choice about what to do. > > Will give it a couple days for feedback before pushing. IMHO it would be better to remove xmin from the lock key, and vacuum away the old predicate locks when the corresponding tuple is vacuumed. The xmin field is only required to handle the case that a tuple is vacuumed, and a new unrelated tuple is inserted to the same slot. Removing the lock when the tuple is removed fixes that. In fact, I cannot even come up with a situation where you would have a problem if we just removed xmin from the key, even if we didn't vacuum away old locks. I don't think the old lock can conflict with anything that would see the new tuple that gets inserted in the same slot. I have a feeling that you could probably prove that if you stare long enough at the diagram of a dangerous structure and the properties required for a conflict. - Heikki
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > IMHO it would be better to remove xmin from the lock key, and vacuum > away the old predicate locks when the corresponding tuple is vacuumed. > The xmin field is only required to handle the case that a tuple is > vacuumed, and a new unrelated tuple is inserted to the same slot. > Removing the lock when the tuple is removed fixes that. > > In fact, I cannot even come up with a situation where you would have a > problem if we just removed xmin from the key, even if we didn't vacuum > away old locks. I don't think the old lock can conflict with anything > that would see the new tuple that gets inserted in the same slot. I have > a feeling that you could probably prove that if you stare long enough at > the diagram of a dangerous structure and the properties required for a > conflict. You are the one who suggested adding xmin to the key: http://www.postgresql.org/message-id/4D5A36FC.6010203@enterprisedb.com I will review that thread in light of your recent comments, but the fact is that xmin was not originally in the lock key, testing uncovered bugs, and adding xmin fixed those bugs. I know I tried some other approach first, which turned out to be complex and quite messy -- it may have been similar to what you are proposing now. It seems to me that a change such as you are now suggesting is likely to be too invasive to back-patch. Do you agree that it would make sense to apply the patch I have proposed, back to 9.1, and then consider any alternative as 9.4 material? -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: > Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > > IMHO it would be better to remove xmin from the lock key, and vacuum > > away the old predicate locks when the corresponding tuple is vacuumed. > > The xmin field is only required to handle the case that a tuple is > > vacuumed, and a new unrelated tuple is inserted to the same slot. > > Removing the lock when the tuple is removed fixes that. This seems definitely safe: we need the predicate locks to determine if someone is modifying a tuple we read, and certainly if it's eligible for vacuum nobody's going to be modifying that tuple anymore. > > In fact, I cannot even come up with a situation where you would have a > > problem if we just removed xmin from the key, even if we didn't vacuum > > away old locks. I don't think the old lock can conflict with anything > > that would see the new tuple that gets inserted in the same slot. I have > > a feeling that you could probably prove that if you stare long enough at > > the diagram of a dangerous structure and the properties required for a > > conflict. This would also be safe, in the sense that it's OK to flag a conflict even if one doesn't exist. I'm not convinced that it isn't possible to have false positives this way. I think it's possible for a tuple to be vacuumed away and the ctid reused before the predicate locks on it are eligible for cleanup. (In fact, isn't this what was happening in the thread Kevin linked?) > You are the one who suggested adding xmin to the key: > > http://www.postgresql.org/message-id/4D5A36FC.6010203@enterprisedb.com > > I will review that thread in light of your recent comments, but the > fact is that xmin was not originally in the lock key, testing > uncovered bugs, and adding xmin fixed those bugs. I know I tried > some other approach first, which turned out to be complex and quite > messy -- it may have been similar to what you are proposing now. At the time, we thought it was necessary for a predicate lock to lock *all future versions* of a tuple, and so we had a bunch of code to maintain a version chain. That was fraught with bugs, and turned out not to be necessary (IIRC, we worked that out at the pub at PGcon). That made it critical to distinguish different tuples that had the same ctid because they could wind up in the wrong chain or cause a cycle. With that code ripped out, that's no longer an issue. But all this is an exceptionally subtle part of what was an exceptionally complex patch, so a lot of careful thought is needed here... > It seems to me that a change such as you are now suggesting is > likely to be too invasive to back-patch. Do you agree that it > would make sense to apply the patch I have proposed, back to 9.1, > and then consider any alternative as 9.4 material? I agree with this. Dan -- Dan R. K. Ports UW CSE http://drkp.net/
On 2013-10-03 21:14:17 -0700, Dan Ports wrote: > On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: > > Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > > > IMHO it would be better to remove xmin from the lock key, and vacuum > > > away the old predicate locks when the corresponding tuple is vacuumed. > > > The xmin field is only required to handle the case that a tuple is > > > vacuumed, and a new unrelated tuple is inserted to the same slot. > > > Removing the lock when the tuple is removed fixes that. > > This seems definitely safe: we need the predicate locks to determine if > someone is modifying a tuple we read, and certainly if it's eligible > for vacuum nobody's going to be modifying that tuple anymore. But we're talking about freezing a tuple, not removing a dead tuple. I don't see anything preventing modification of a frozen tuple. Right? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 04.10.2013 13:23, Andres Freund wrote: > On 2013-10-03 21:14:17 -0700, Dan Ports wrote: >> On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: >>> Heikki Linnakangas<hlinnakangas@vmware.com> wrote: >>>> IMHO it would be better to remove xmin from the lock key, and vacuum >>>> away the old predicate locks when the corresponding tuple is vacuumed. >>>> The xmin field is only required to handle the case that a tuple is >>>> vacuumed, and a new unrelated tuple is inserted to the same slot. >>>> Removing the lock when the tuple is removed fixes that. >> >> This seems definitely safe: we need the predicate locks to determine if >> someone is modifying a tuple we read, and certainly if it's eligible >> for vacuum nobody's going to be modifying that tuple anymore. > > But we're talking about freezing a tuple, not removing a dead tuple. I > don't see anything preventing modification of a frozen tuple. Right? Right, but if we no longer include xmin in the lock key, freezing a tuple makes no difference. Currently, the problem is that when a tuple is frozen, the locks on the tuple on the tuple are "lost", as the xmin+ctid of the lock no longer matches xmin+ctid of the tuple. Removing xmin from the lock altogether solves that problem, but it introduces the possibility that when an old tuple is vacuumed away and a new tuple is inserted on the same slot, a lock on the old tuple is confused to be a lock on the new tuple. And that problem can be fixed by vacuuming locks, when the corresponding tuple is vacuumed. - Heikki
On 2013-10-04 13:51:00 +0300, Heikki Linnakangas wrote: > On 04.10.2013 13:23, Andres Freund wrote: > >On 2013-10-03 21:14:17 -0700, Dan Ports wrote: > >>On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: > >>>Heikki Linnakangas<hlinnakangas@vmware.com> wrote: > >>>>IMHO it would be better to remove xmin from the lock key, and vacuum > >>>>away the old predicate locks when the corresponding tuple is vacuumed. > >>>>The xmin field is only required to handle the case that a tuple is > >>>>vacuumed, and a new unrelated tuple is inserted to the same slot. > >>>>Removing the lock when the tuple is removed fixes that. > >> > >>This seems definitely safe: we need the predicate locks to determine if > >>someone is modifying a tuple we read, and certainly if it's eligible > >>for vacuum nobody's going to be modifying that tuple anymore. > > > >But we're talking about freezing a tuple, not removing a dead tuple. I > >don't see anything preventing modification of a frozen tuple. Right? > > Right, but if we no longer include xmin in the lock key, freezing a tuple > makes no difference. Currently, the problem is that when a tuple is frozen, > the locks on the tuple on the tuple are "lost", as the xmin+ctid of the lock > no longer matches xmin+ctid of the tuple. > > Removing xmin from the lock altogether solves that problem, but it > introduces the possibility that when an old tuple is vacuumed away and a new > tuple is inserted on the same slot, a lock on the old tuple is confused to > be a lock on the new tuple. And that problem can be fixed by vacuuming > locks, when the corresponding tuple is vacuumed. But locks on those still can have meaning, right? From my very limited understanding of predicate.c/SSI I don't see why we cannot have meaningful conflicts on tuples that are already vacuumable. You'd need some curiously interleaved transactions, sure, but it seems possible? ISTM we'd need to peg the xmin horizon for vacuum to the oldest xmin predicate.c keeps track of. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 04.10.2013 14:02, Andres Freund wrote: > But locks on those still can have meaning, right? From my very limited > understanding of predicate.c/SSI I don't see why we cannot have > meaningful conflicts on tuples that are already vacuumable. You'd need > some curiously interleaved transactions, sure, but it seems possible? To conflict with a lock, a backend would need to read or update the tuple the lock is on. If the tuple is vacuumable, it's no longer visible to anyone, so no backend can possibly read or update it anymore. - Heikki
Andres Freund <andres@2ndquadrant.com> wrote: >>> On 2013-10-03 21:14:17 -0700, Dan Ports wrote: >>>>> Heikki Linnakangas<hlinnakangas@vmware.com> wrote: >>>>>> IMHO it would be better to remove xmin from the lock key, >>>>>> and vacuum away the old predicate locks when the >>>>>> corresponding tuple is vacuumed. >>>>>> The xmin field is only required to handle the case that a >>>>>> tuple is vacuumed, and a new unrelated tuple is inserted to >>>>>> the same slot. >>>>>> Removing the lock when the tuple is removed fixes that. >>>> >>>> This seems definitely safe: we need the predicate locks to >>>> determine if someone is modifying a tuple we read, and >>>> certainly if it's eligible for vacuum nobody's going to be >>>> modifying that tuple anymore. I totally agree. It would be safe, and generally seems better, to omit xmin from the hash tag for heap tuple locks. > But locks on those still can have meaning, right? From my very > limited understanding of predicate.c/SSI I don't see why we > cannot have meaningful conflicts on tuples that are already > vacuumable. You'd need some curiously interleaved transactions, > sure, but it seems possible? No. We established quite firmly that predicate locks on a tuple do not need to be carried forward to new versions of that tuple. It is only the *version* of the row that was read to create the predicate lock which needs to be monitored for write conflicts. If the row has been vacuumed, or even determined to be eligible for vacuuming, we know it is not a candidate for update or delete -- so it is safe to drop the predicate locks. We do *not* want to do any other cleanup for the transaction which read that tuple based on this; just the individual tuple lock should be cleared. Any conflict with the lock which occurred earlier will be preserved. > ISTM we'd need to peg the xmin horizon for vacuum to the oldest > xmin predicate.c keeps track of. That would be another alternative, but it seems more problematic and less effective. I'm strongly leaning toward the idea that a slightly tweaked version of the proposed patch is appropriate for the back-branches, because the fix Heikki is now suggesting seems too invasive to back-patch. I think it would make sense to apply it to master, too, so that the new isolation tests can be immediately added. We can then work on the new approach for 9.4 and have the tests to help confirm we are not breaking anything. The tweak would be to preserve the signature of heap_freeze_tuple(), because after the more invasive fix in 9.4 the new parameters will not be needed. They are only passed as non-NULL from one of the three callers, so it seems best to leave those call sites alone rather than change them back-and-forth. I will post a new patch today or tomorrow. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Kevin Grittner <kgrittn@ymail.com> wrote: > I'm strongly leaning toward the idea that a slightly tweaked > version of the proposed patch is appropriate for the back-branches, > because the fix Heikki is now suggesting seems too invasive to > back-patch. I think it would make sense to apply it to master, > too, so that the new isolation tests can be immediately added. We > can then work on the new approach for 9.4 and have the tests to > help confirm we are not breaking anything. The tweak would be to > preserve the signature of heap_freeze_tuple(), because after the > more invasive fix in 9.4 the new parameters will not be needed. > They are only passed as non-NULL from one of the three callers, so > it seems best to leave those call sites alone rather than change > them back-and-forth. > > I will post a new patch today or tomorrow. Attached. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Kevin Grittner <kgrittn@ymail.com> wrote: > Kevin Grittner <kgrittn@ymail.com> wrote: > >> I'm strongly leaning toward the idea that a slightly tweaked >> version of the proposed patch is appropriate for the back-branches, >> because the fix Heikki is now suggesting seems too invasive to >> back-patch. I think it would make sense to apply it to master, >> too, so that the new isolation tests can be immediately added. We >> can then work on the new approach for 9.4 and have the tests to >> help confirm we are not breaking anything. The tweak would be to >> preserve the signature of heap_freeze_tuple(), because after the >> more invasive fix in 9.4 the new parameters will not be needed. >> They are only passed as non-NULL from one of the three callers, so >> it seems best to leave those call sites alone rather than change >> them back-and-forth. >> >> I will post a new patch today or tomorrow. > > Attached. And here's a rough cut of what I think the alternative now suggested by Heikki would look like. (I've omitted the new isolation test because that is the same as the previously posted patch.) Note this comment, which I think was written by Heikki back when there was a lot more benchmarking and profiling of SSI going on: * Because a particular target might become obsolete, due to update to a new * version, before the reading transaction is obsolete, we need some way to * prevent errors from reuse of a tuple ID. Rather than attempting to clean * up the targets as the related tuples are pruned or vacuumed, we check the * xmin on access. This should be far less costly. Based on what this patch looks like, I'm afraid he may have been right when he wrote that. In any event, after the exercise of developing a first draft of searching for predicate locks to clean up every time a tuple is pruned or vacuumed, I continue to feel strongly that the previously-posted patch, which only takes action when tuples are frozen by a vacuum process, is much more suitable for backpatching. I don't think we should switch to anything resembling the attached without some careful benchmarking. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On 06.10.2013 20:34, Kevin Grittner wrote: > Note this comment, which I think was written by Heikki back when > there was a lot more benchmarking and profiling of SSI going on: > > * Because a particular target might become obsolete, due to update to a new > * version, before the reading transaction is obsolete, we need some way to > * prevent errors from reuse of a tuple ID. Rather than attempting to clean > * up the targets as the related tuples are pruned or vacuumed, we check the > * xmin on access. This should be far less costly. > > Based on what this patch looks like, I'm afraid he may have been > right when he wrote that. In any event, after the exercise of > developing a first draft of searching for predicate locks to clean > up every time a tuple is pruned or vacuumed, I continue to feel > strongly that the previously-posted patch, which only takes action > when tuples are frozen by a vacuum process, is much more suitable > for backpatching. I don't think we should switch to anything > resembling the attached without some careful benchmarking. Hmm, you're probably right. I was thinking that the overhead of scanning the lock hash on a regular vacuum is negligible, but I didn't consider pruning. It might be significant there. I'd like to give this line of investigation some more thought: > On 04.10.2013 07:14, Dan Ports wrote: >> On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: >>> Heikki Linnakangas<hlinnakangas@vmware.com> wrote: >>>> IMHO it would be better to remove xmin from the lock key, and vacuum >>>> away the old predicate locks when the corresponding tuple is vacuumed. >>>> The xmin field is only required to handle the case that a tuple is >>>> vacuumed, and a new unrelated tuple is inserted to the same slot. >>>> Removing the lock when the tuple is removed fixes that. >> >> This seems definitely safe: we need the predicate locks to determine if >> someone is modifying a tuple we read, and certainly if it's eligible >> for vacuum nobody's going to be modifying that tuple anymore. >> >>>> In fact, I cannot even come up with a situation where you would have a >>>> problem if we just removed xmin from the key, even if we didn't vacuum >>>> away old locks. I don't think the old lock can conflict with anything >>>> that would see the new tuple that gets inserted in the same slot. I have >>>> a feeling that you could probably prove that if you stare long enough at >>>> the diagram of a dangerous structure and the properties required for a >>>> conflict. >> >> This would also be safe, in the sense that it's OK to flag a >> conflict even if one doesn't exist. I'm not convinced that it isn't >> possible to have false positives this way. I think it's possible for a >> tuple to be vacuumed away and the ctid reused before the predicate >> locks on it are eligible for cleanup. (In fact, isn't this what was >> happening in the thread Kevin linked?) Time to stare at the dangerous structure again: > SSI is based on the observation [2] that each snapshot isolation > anomaly corresponds to a cycle that contains a "dangerous structure" > of two adjacent rw-conflict edges: > > Tin ------> Tpivot ------> Tout > rw rw > Furthermore, Tout must commit first. The following are true: * A transaction can only hold a predicate lock on a tuple that's visible to it. * A tuple that's visible to Tin or Tpivot cannot be vacuumed away until Tout commits. When updating a tuple, CheckTargetForConflictsIn() only marks a conflict if the transaction holding the predicate lock overlapped with the updating transaction. And if a tuple is vacuumed away and the slot is reused, an transaction updating the new tuple cannot overlap with any transaction holding a lock on the old tuple. Otherwise the tuple wouldn't have been eligible for vacuuming in the first place. So I don't think you can ever get a false conflict because of slot reuse. And if there's a hole in that thinking I can't see right now, the worst that will happen is some unnecessary conflicts, ie. it's still correct. It surely can't be worse than upgrading the lock to a page-level lock, which would also create unnecessary conflicts. Summary: IMHO we should just remove xmin from the predicate lock tag. - Heikki
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > So I don't think you can ever get a false conflict because of > slot reuse. I spent some time looking at this, and I now agree. > And if there's a hole in that thinking I can't see right now, the > worst that will happen is some unnecessary conflicts, ie. it's > still correct. That is definitely true; no doubt about that part. > Summary: IMHO we should just remove xmin from the predicate lock > tag. I spent some time trying to see how the vacuum could happen at a point that could cause a false positive, and was unable to do so. Even if that is just a failure of imagination on my part, the above argument that it doesn't produce incorrect results still holds. I think the fact that I couldn't find a sequence of events which would trigger a false positive suggests it would be a rare case, anyway. I found the original bug report which led to the addition of xmin to the tag, and it was because we were still carrying tuple locks forward to new versions of those locks at the time. This was later proven to be unnecessary, which simplified other code; we apparently missed a trick in not removing xmin from the lock tag at that point. Since leaving it there has now been shown to *cause* a bug, I'm inclined to agree that we should simply remove it, and backpatch that. Patch attached. Any objections to applying that Real Soon Now? (When, exactly is the deadline to make today's minor release cut-off?) -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Kevin Grittner <kgrittn@ymail.com> wrote: > Patch attached. Any objections to applying that Real Soon Now? Oh, without the new line in predicate.h. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2013-10-07 06:44:19 -0700, Kevin Grittner wrote: > Patch attached. Any objections to applying that Real Soon Now? > (When, exactly is the deadline to make today's minor release > cut-off?) Maybe it's overly careful, but I personally slightly vote for applying it after the backbranch releases. The current behaviour doesn't have any harsh consequences and mostly reproduceable in artifical scenarios and the logic here is complex enough that we might miss something. A day just doesn't leave much time to noticing any issues. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 07.10.2013 16:58, Andres Freund wrote: > On 2013-10-07 06:44:19 -0700, Kevin Grittner wrote: >> Patch attached. Any objections to applying that Real Soon Now? >> (When, exactly is the deadline to make today's minor release >> cut-off?) > > Maybe it's overly careful, but I personally slightly vote for applying > it after the backbranch releases. The current behaviour doesn't have any > harsh consequences and mostly reproduceable in artifical scenarios and > the logic here is complex enough that we might miss something. Well, it's fairly harsh if the feature isn't working as advertised. > A day just doesn't leave much time to noticing any issues. It's less than ideal, I agree, but it doesn't seem better to me to just leave the bug unfixed for another minor release. Even if we sit on this for another few months, I don't think we'll get any meaningful amount of extra testing on it. - Heikki
On 07.10.2013 16:44, Kevin Grittner wrote: > Heikki Linnakangas<hlinnakangas@vmware.com> wrote: > >> So I don't think you can ever get a false conflict because of >> slot reuse. > > I spent some time looking at this, and I now agree. > >> And if there's a hole in that thinking I can't see right now, the >> worst that will happen is some unnecessary conflicts, ie. it's >> still correct. > > That is definitely true; no doubt about that part. > >> Summary: IMHO we should just remove xmin from the predicate lock >> tag. > > I spent some time trying to see how the vacuum could happen at a > point that could cause a false positive, and was unable to do so. > Even if that is just a failure of imagination on my part, the above > argument that it doesn't produce incorrect results still holds. I > think the fact that I couldn't find a sequence of events which > would trigger a false positive suggests it would be a rare case, > anyway. > > I found the original bug report which led to the addition of xmin > to the tag, and it was because we were still carrying tuple locks > forward to new versions of those locks at the time. This was later > proven to be unnecessary, which simplified other code; we > apparently missed a trick in not removing xmin from the lock tag at > that point. Since leaving it there has now been shown to *cause* a > bug, I'm inclined to agree that we should simply remove it, and > backpatch that. > > Patch attached. Any objections to applying that Real Soon Now? > (When, exactly is the deadline to make today's minor release > cut-off?) A comment somewhere would be nice to explain why we're no longer worried about confusing an old tuple version with a new tuple in the same slot. Not sure where. - Heikki
On 2013-10-07 17:07:16 +0300, Heikki Linnakangas wrote: > On 07.10.2013 16:58, Andres Freund wrote: > >On 2013-10-07 06:44:19 -0700, Kevin Grittner wrote: > >>Patch attached. Any objections to applying that Real Soon Now? > >>(When, exactly is the deadline to make today's minor release > >>cut-off?) > > > >Maybe it's overly careful, but I personally slightly vote for applying > >it after the backbranch releases. The current behaviour doesn't have any > >harsh consequences and mostly reproduceable in artifical scenarios and > >the logic here is complex enough that we might miss something. > > Well, it's fairly harsh if the feature isn't working as advertised. Well, you need to have a predicate lock on a tuple that's getting frozen (i.e. hasn't been written to for 50mio+ xids) and you need to have an update during the time that predicate lock is held. That's not too likely without explicit VACUUM FREEZEs interleaved there somewhere. > >A day just doesn't leave much time to noticing any issues. > > It's less than ideal, I agree, but it doesn't seem better to me to just > leave the bug unfixed for another minor release. Even if we sit on this for > another few months, I don't think we'll get any meaningful amount of extra > testing on it. Yea, maybe. Although HEAD does get some testing... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > Well, it's fairly harsh if the feature isn't working as > advertised. Right -- there are people counting on serializable transaction to avoid data corruption (creation of data which violates the business rules which they believe are being enforced). A tuple freeze at the wrong moment could currently break that. The patch allows the guarantee to hold. The fact that this bug is only seen if there is a very-long-running transaction or if VACUUM FREEZE is run while data-modifying transactions are active does not eliminate the fact that without this patch data can be corrupted. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Andres Freund <andres@2ndquadrant.com> wrote: > On 2013-10-07 06:44:19 -0700, Kevin Grittner wrote: > >> Patch attached. Any objections to applying that Real Soon Now? >> (When, exactly is the deadline to make today's minor release >> cut-off?) > > Maybe it's overly careful, but I personally slightly vote for applying > it after the backbranch releases. The current behaviour doesn't have any > harsh consequences and mostly reproduceable in artifical scenarios and > the logic here is complex enough that we might miss something. > > A day just doesn't leave much time to noticing any issues. I grant that the bug in existing production code is not likely to get hit very often, but it is a bug; the new isolation test shows the bug clearly and shows that the suggested patch fixes it. What tips the scales for me is that the only possible downside if we missed something is an occasional false positive serialization failure, which does not break correctness -- we try to minimize those for performance reasons, but the algorithm allows them and they currently do happen. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Kevin Grittner <kgrittn@ymail.com> wrote: > Andres Freund <andres@2ndquadrant.com> wrote: >> On 2013-10-07 06:44:19 -0700, Kevin Grittner wrote: >> >>> Patch attached. Any objections to applying that Real Soon Now? >>> (When, exactly is the deadline to make today's minor release >>> cut-off?) >> >> Maybe it's overly careful, but I personally slightly vote for >> applying it after the backbranch releases. The current behaviour >> doesn't have any harsh consequences and mostly reproduceable in >> artifical scenarios and the logic here is complex enough that we >> might miss something. >> >> A day just doesn't leave much time to noticing any issues. > > I grant that the bug in existing production code is not likely to > get hit very often, but it is a bug; the new isolation test shows > the bug clearly and shows that the suggested patch fixes it. > What tips the scales for me is that the only possible downside if > we missed something is an occasional false positive serialization > failure, which does not break correctness -- we try to minimize > those for performance reasons, but the algorithm allows them and > they currently do happen. I am, of course, continuing to review this. There might be a problem if someone applies this fix while any prepared transactions are pending. Still investigating the impact and possible fixes. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Kevin Grittner <kgrittn@ymail.com> wrote: > There might be a problem if someone applies this fix while any > prepared transactions are pending. Still investigating the impact > and possible fixes. I found a one-line fix for this. It tested without problem. Pushed. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Oct 07, 2013 at 12:26:37PM +0300, Heikki Linnakangas wrote: > When updating a tuple, CheckTargetForConflictsIn() only marks a > conflict if the transaction holding the predicate lock overlapped > with the updating transaction. Ah, this is the bit I was forgetting. (I really ought to have remembered that, but it's been a while...) I think it's possible, then, to construct a scenario where a slot is reused before predicate locks on the old tuple are eligible for cleanup -- but those locks will never cause a conflict. So I agree: it's correct to just remove the xmin from the key unconditionally. And this is also true: > And if there's a hole in that thinking I can't see right now, > the worst that will happen is some unnecessary conflicts, ie. it's > still correct. It surely can't be worse than upgrading the lock to a > page-level lock, which would also create unnecessary conflicts. Dan -- Dan R. K. Ports UW CSE http://drkp.net/
On 07.10.2013 22:35, Kevin Grittner wrote: > Kevin Grittner<kgrittn@ymail.com> wrote: > >> There might be a problem if someone applies this fix while any >> prepared transactions are pending. Still investigating the impact >> and possible fixes. > > I found a one-line fix for this. It tested without problem. Good catch. To fix the bug that Hannu pointed out, we also need to apply these fixes: http://www.postgresql.org/message-id/52440266.5040708@vmware.com - Heikki
On 07.10.2013 23:45, Heikki Linnakangas wrote: > On 07.10.2013 22:35, Kevin Grittner wrote: >> Kevin Grittner<kgrittn@ymail.com> wrote: >> >>> There might be a problem if someone applies this fix while any >>> prepared transactions are pending. Still investigating the impact >>> and possible fixes. >> >> I found a one-line fix for this. It tested without problem. > > Good catch. > > To fix the bug that Hannu pointed out, we also need to apply these fixes: > > http://www.postgresql.org/message-id/52440266.5040708@vmware.com Per a chat with Bruce, I'm going to apply that patch now to get it into the minor releases. - Heikki
On Mon, 2013-10-07 at 23:49 +0300, Heikki Linnakangas wrote: > On 07.10.2013 23:45, Heikki Linnakangas wrote: > > On 07.10.2013 22:35, Kevin Grittner wrote: > >> Kevin Grittner<kgrittn@ymail.com> wrote: > >> > >>> There might be a problem if someone applies this fix while any > >>> prepared transactions are pending. Still investigating the impact > >>> and possible fixes. > >> > >> I found a one-line fix for this. It tested without problem. > > > > Good catch. > > > > To fix the bug that Hannu pointed out, we also need to apply these fixes: > > > > http://www.postgresql.org/message-id/52440266.5040708@vmware.com > > Per a chat with Bruce, I'm going to apply that patch now to get it into > the minor releases. 9.1 branch is broken now.
On 08.10.2013 03:25, Peter Eisentraut wrote: > On Mon, 2013-10-07 at 23:49 +0300, Heikki Linnakangas wrote: >>> To fix the bug that Hannu pointed out, we also need to apply these fixes: >>> >>> http://www.postgresql.org/message-id/52440266.5040708@vmware.com >> >> Per a chat with Bruce, I'm going to apply that patch now to get it into >> the minor releases. > > 9.1 branch is broken now Rats. I forgot to "git add" the latest changes while backpatching. Committed a fix for that. - Heikki
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > A comment somewhere would be nice to explain why we're no longer worried > about confusing an old tuple version with a new tuple in the same slot. > Not sure where. For now I counted on the commit message. Perhaps we also want to add something to the README file? -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > On 07.10.2013 23:45, Heikki Linnakangas wrote: >> To fix the bug that Hannu pointed out, we also need to apply these fixes: >> >> http://www.postgresql.org/message-id/52440266.5040708@vmware.com > > Per a chat with Bruce, I'm going to apply that patch now to get it into > the minor releases. Please do. (Somehow I had it in my head that you already had done so.) -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company