On Tue, Jun 21, 2011 at 03:01:48PM +0300, Heikki Linnakangas wrote:
> Thanks, committed.
Thanks.
> In the long term, I'm not sure this is the best way to handle this. It
> feels a bit silly to set the flag, release the SerializableXactHashLock,
> and reacquire it later to remove the transaction from the hash table.
> Surely that could be done in some more straightforward way. But I don't
> feel like fiddling with it this late in the release cycle.
Yes, I suspect it can be done better. The reason it's tricky is a lock
ordering issue; part of releasing a SerializableXact has to be done
while holding SerializableXactHashLock and part has to be done without
it (because it involves taking partition locks). Reworking the order of
these things might work, but would require some careful thought since
most of the code is shared with the non-abort cleanup paths. And yes,
it's definitely the time for that.
I've been meaning to take another look at this part of the code anyway,
with an eye towards performance. SerializableXactHashLock can be a
bottleneck on certain in-memory workloads.
> > One of the prepared_xacts regression tests actually hits this bug.
> > I removed the anomaly from the duplicate-gids test so that it fails in
> > the intended way, and added a new test to check serialization failures
> > with a prepared transaction.
>
> Hmm, I have ran "make installcheck" with
> default_transaction_isolation='serializable' earlier. I wonder why I
> didn't see that.
The affected test was being run at SERIALIZABLE already, so that
wouldn't have made a difference. One reason I didn't notice an issue
when I looked at that test before before, is that it was intended to
fail anyway, just with a different error. I didn't think too hard about
which error would take precedence.
Dan
--
Dan R. K. Ports MIT CSAIL http://drkp.net/