On 23/03/16 19:39, Robert Haas wrote:
> On Tue, Mar 22, 2016 at 1:12 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> I also think the code simplicity makes this worth it.
>
> Agreed. I went over this patch and did a cleanup pass today. I
> discovered that the LockWaiterCount() function was broken if you try
> to tried to use it on a lock that you didn't hold or a lock that you
> held in any mode other than exclusive, so I tried to fix that. I
> rewrote a lot of the comments and tightened some other things up. The
> result is attached.
>
> I'm baffled by the same code Amit asked about upthread, even though
> there's now a comment:
>
> + /*
> + * Here we are calling
> RecordAndGetPageWithFreeSpace
> + * instead of GetPageWithFreeSpace,
> because other backend
> + * who have got the lock might have
> added extra blocks in
> + * the FSM and its possible that FSM
> tree might not have
> + * been updated so far and we will not
> get the page by
> + * searching from top using
> GetPageWithFreeSpace, so we
> + * need to search the leaf page
> directly using our last
> + * valid target block.
> + *
> + * XXX. I don't understand what is
> happening here. -RMH
> + */
>
> I've read this over several times and looked at
> RecordAndGetPageWithFreeSpace() and I'm still confused. First of all,
> if the lock was acquired by some other backend which did
> RelationAddExtraBlocks(), it *will* have updated the FSM - that's the
> whole point.
That's good point, maybe this coding is bit too defensive.
> Second, if the other backend extended the relation in
> some other manner and did not extend the FSM, how does calling
> RecordAndGetPageWithFreeSpace help? As far as I can see,
> GetPageWithFreeSpace and RecordAndGetPageWithFreeSpace are both just
> searching the FSM, so if one is stymied the other will be too. What
> am I missing?
>
The RecordAndGetPageWithFreeSpace will extend FSM as it calls
fsm_set_and_search which in turn calls fsm_readbuf with extend = true.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services