On 01/11/2014 12:39 PM, Peter Geoghegan wrote:
In any case, my patch is bound to win decisively for the other extreme, the insert-only case, because the overhead of doing an index scan first is always wasted there with your approach, and the overhead of extended btree leaf page locking has been shown to be quite low.
Quite possibly. Run the benchmark, and we'll see how big a difference we're talking about.
In the past you've spoken of avoiding that overhead through an adaptive strategy based on statistics, but I think you'll have a hard time beating a strategy where the decision comes as late as possible, and is informed by highly localized page-level metadata already available. My implementation can abort an attempt to just read an existing would-be duplicate very inexpensively (with no strong locks), going back to just after the _bt_search() to get a heavyweight lock if just reading doesn't work out (if there is no duplicate found), so as to not waste all of its prior work. Doing one of the two extremes of insert-mostly or update-only well is relatively easy; dynamically adapting to one or the other is much harder. Especially if it's a consistent mix of inserts and updates, where general observations aren't terribly useful.
Another way to optimize it is to keep the b-tree page pinned after doing the pre-check. Then you don't need to descend the tree again when doing the insert. That would require small indexam API changes, but wouldn't be too invasive, I think.
All other concerns of mine still remain, including the concern over the extra locking of the proc array - I'm concerned about the performance impact of that on other parts of the system not exercised by this test.
Yeah, I'm not thrilled about that part either. Fortunately there are other ways to implement that. In fact, I think you could just not bother taking the ProcArrayLock when setting the fields. The danger is that another backend sees a mixed state of the fields, but that's OK. The worst that can happen is that it will do an unnecessary lock/release on the heavy-weight lock. And to reduce the overhead when reading the fields, you could merge the SpeculativeInsertionIsInProgress() check into TransactionIdIsInProgress(). The call site in tqual.c always calls it together with TransactionIdIsInProgress(), which scans the proc array anyway, while holding the lock.
- Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
