Thread
-
Re: Bugs in CREATE/DROP INDEX CONCURRENTLY
Kevin Grittner <kgrittn@mail.com> — 2012-10-18T04:12:02Z
Kevin Grittner wrote: > Hmm. The comment is probably better now, but I've been re-checking > the code, and I think my actual code change is completely wrong. > Give me a bit to sort this out. I'm having trouble seeing a way to make this work without rearranging the code for concurrent drop to get to a state where it has set indisvalid = false, made that visible to all processes, and ensured that all scans of the index are complete -- while indisready is still true. That is the point where TransferPredicateLocksToHeapRelation() could be safely called. Then we would need to set indisready = false, make that visible to all processes, and ensure that all access to the index is complete. I can't see where it works to set both flags at the same time. I want to sleep on it to see if I can come up with any other way, but right now that's the only way I'm seeing to make DROP INDEX CONCURRENTLY compatible with SERIALIZABLE transactions. :-( -Kevin
-
Re: Bugs in CREATE/DROP INDEX CONCURRENTLY
Andres Freund <andres@2ndquadrant.com> — 2012-10-18T09:20:23Z
On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: > Kevin Grittner wrote: > > Hmm. The comment is probably better now, but I've been re-checking > > the code, and I think my actual code change is completely wrong. > > Give me a bit to sort this out. > > I'm having trouble seeing a way to make this work without rearranging > the code for concurrent drop to get to a state where it has set > indisvalid = false, made that visible to all processes, and ensured > that all scans of the index are complete -- while indisready is still > true. That is the point where TransferPredicateLocksToHeapRelation() > could be safely called. Then we would need to set indisready = false, > make that visible to all processes, and ensure that all access to the > index is complete. I can't see where it works to set both flags at > the same time. I want to sleep on it to see if I can come up with any > other way, but right now that's the only way I'm seeing to make DROP > INDEX CONCURRENTLY compatible with SERIALIZABLE transactions. :-( In a nearby bug I had to restructure the code that in a way thats similar to this anyway, so that seems fine. Maybe you can fix the bug ontop of the two attached patches? Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
-
Re: Bugs in CREATE/DROP INDEX CONCURRENTLY
Simon Riggs <simon@2ndquadrant.com> — 2012-10-18T18:48:31Z
On 18 October 2012 10:20, Andres Freund <andres@2ndquadrant.com> wrote: > On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >> Kevin Grittner wrote: >> > Hmm. The comment is probably better now, but I've been re-checking >> > the code, and I think my actual code change is completely wrong. >> > Give me a bit to sort this out. >> >> I'm having trouble seeing a way to make this work without rearranging >> the code for concurrent drop to get to a state where it has set >> indisvalid = false, made that visible to all processes, and ensured >> that all scans of the index are complete -- while indisready is still >> true. That is the point where TransferPredicateLocksToHeapRelation() >> could be safely called. Then we would need to set indisready = false, >> make that visible to all processes, and ensure that all access to the >> index is complete. I can't see where it works to set both flags at >> the same time. I want to sleep on it to see if I can come up with any >> other way, but right now that's the only way I'm seeing to make DROP >> INDEX CONCURRENTLY compatible with SERIALIZABLE transactions. :-( > > In a nearby bug I had to restructure the code that in a way thats similar to > this anyway, so that seems fine. Maybe you can fix the bug ontop of the two > attached patches? First patch and first test committed. Working on second patch/test. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
-
Re: Bugs in CREATE/DROP INDEX CONCURRENTLY
Simon Riggs <simon@2ndquadrant.com> — 2012-10-19T09:10:23Z
On 18 October 2012 19:48, Simon Riggs <simon@2ndquadrant.com> wrote: > On 18 October 2012 10:20, Andres Freund <andres@2ndquadrant.com> wrote: >> On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >>> Kevin Grittner wrote: >>> > Hmm. The comment is probably better now, but I've been re-checking >>> > the code, and I think my actual code change is completely wrong. >>> > Give me a bit to sort this out. >>> >>> I'm having trouble seeing a way to make this work without rearranging >>> the code for concurrent drop to get to a state where it has set >>> indisvalid = false, made that visible to all processes, and ensured >>> that all scans of the index are complete -- while indisready is still >>> true. That is the point where TransferPredicateLocksToHeapRelation() >>> could be safely called. Then we would need to set indisready = false, >>> make that visible to all processes, and ensure that all access to the >>> index is complete. I can't see where it works to set both flags at >>> the same time. I want to sleep on it to see if I can come up with any >>> other way, but right now that's the only way I'm seeing to make DROP >>> INDEX CONCURRENTLY compatible with SERIALIZABLE transactions. :-( >> >> In a nearby bug I had to restructure the code that in a way thats similar to >> this anyway, so that seems fine. Maybe you can fix the bug ontop of the two >> attached patches? > > First patch and first test committed. > > Working on second patch/test. I've applied the second patch as-is. The second test shows it passes, but the nature of the bug is fairly obscure, so having a specific test for dropping an already dropped object is a little strange and so I've not applied that. Thanks for fixes and tests. Kevin, you're good to go on the SSI patch, or I'll apply next week if you don't. Thanks for that. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services