On 06.03.2012 14:52, Fujii Masao wrote:
> On Tue, Mar 6, 2012 at 2:17 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:
>>> On 21.02.2012 13:19, Fujii Masao wrote:
>>>> In some places, the spinlock "insertpos_lck" is taken while another
>>>> spinlock "info_lck" is being held. Is this OK? What if unfortunately
>>>> inner spinlock takes long to be taken?
>>
>>> Hmm, that's only done at a checkpoint (and a restartpoint), so I doubt
>>> that's a big issue in practice. We had the same pattern before the
>>> patch, just with WALInsertLock instead of insertpos_lck. Holding a
>>> spinlock longer is much worse than holding a lwlock longer, but
>>> nevertheless I don't think that's a problem.
>>
>> No, that's NOT okay. A spinlock is only supposed to be held across a
>> short straight-line sequence of instructions.
Ok, that's easy enough to fix.
> This also strikes me that the usage of the spinlock insertpos_lck might
> not be OK in ReserveXLogInsertLocation() because a few dozen instructions
> can be performed while holding the spinlock....
I admit that block is longer than any of our existing spinlock blocks.
However, it's important for performance. I tried using a lwlock earlier,
and that negated the gains. So if that's a serious objection, then let's
resolve that now before I spend any more time on other aspects of the
patch. Any ideas how to make that block shorter?
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com