On 23/03/16 19:39, Robert Haas wrote:
On Tue, Mar 22, 2016 at 1:12 PM, Petr Jelinek <p...@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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to