On Fri, Sep 26, 2014 at 5:24 AM, Heikki Linnakangas <hlinnakan...@vmware.com> wrote: > Just to be clear: I wrote the initial patch to demonstrate what I had in > mind, because I was not able to explain it well enough otherwise. You > pointed out issues with it, which I then fixed. You then pointed out more > issues, which I then fixed again.
> My patch version was a proof of concept, to demonstrate that it can be done. Right. It was a rough prototype built to prove a point. It also served to show what I was talking about as regards deadlocks (and how the locks could problematically persist in other ways), which I was previously unable to effectively explain to Andres. So it was a very useful exercise, and I wish we did that kind of thing more frequently. But at the same time, I don't want to hold you to that prototype, or misrepresent that prototype as showing your final position on any technical issue. So please correct me if I do that. I've tried to be careful about that. > What I'd like you to do now, as the patch author, is to take the promise > tuple approach and clean it up. If the xmin stuff is ugly, figure out some > other way to do it. My concern with the xmin stuff is not that it's ugly; it's that it's potentially dangerous. It isn't at all easy to reason about where bugs might appear - lots of things could interact with it in unpredictable ways. I think we'd have to audit a lot of code, all over the place, just to make sure nowhere had an assumption broken. This is a big issue. You are asking me to find a way to save a design that I don't particularly believe in. That might change, but right now I'm afraid that that's the reality. Whereas, my design is entirely contained in the file nbtinsert.c. > I don't know what you mean by "in the head of AM", but IMO it would be far > better if we can implement this outside the index AMs. Then it will work > with any index AM. I mean that "value locking" is an abstraction that lives in the head of amcanunique AMs. That kind of encapsulation has considerable value in reducing the risk of bugs. If what I've done has bugs, there isn't that many places that could expose interactions with other complicated code. There are fewer moving parts. It's a generalization of the existing mechanism for unique index enforcement. Plus, database systems have used heavyweight index locks for this kind of thing since the 1970s. That's how this works everywhere else (SQL server certainly does this for MERGE [1], and only grabs the page-level lock for a second at lower isolation levels, as in my implementation). I think that that ought to count for something. I will be frank. Everyone knows that the nbtree locking parts of this are never going to be committed over your objections. It cannot happen. And yet, I persist in proposing that we go that way. I may be stubborn, but I am not so stubborn that I'd jeopardize all the work I've put into this to save one aspect of it that no one really cares about anyway (even I only care about meeting my goals for user visible behavior [2]). I may actually come up with a better way to make what you outline work; then again, I may not. I have no idea, to be honest. It's pretty clear that I'm going to have a hard time getting your basic approach to value locking accepted without rethinking it a lot, though. Can you really say that you won't have serious misgivings about something like the "tuple->xmin = InvalidTransactionId" swapping, if I actually formally propose it? That's very invasive to a lot of places. And right now, I have no idea how we could do better. I really only want to get to where we have a design that's acceptable. In all sincerity, I may yet be convinced to go your way. It's possible that I've failed to fully understand your concerns. Is it really just about making INSERT ... ON CONFLICT IGNORE work with exclusion constraints (UPDATE clearly makes little sense)? > Basically, an INSERT to a table with an exclusion constraint would be the > same as "INSERT ON CONFLICT throw an error". That would be a useful way to > split this patch into two parts. I'll think about it. I don't want to do that until I see a way to make your approach to value locking work in a way that someone is actually going to be comfortable committing. I am looking for one. By the way, IMO stress testing has a very useful role to play in the development of this feature. I've been doing things like trying to flush out races by running long stress tests with random delays artificially added at key points. I would like to make that part of the testing strategy public and transparent. [1] http://weblogs.sqlteam.com/mladenp/archive/2007/08/03/60277.aspx [2] Slide 8, "Goals for UPSERT in Postgres": http://www.pgcon.org/2014/schedule/attachments/327_upsert_weird.pdf -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
