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. 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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company