Re: Relation extension scalability

On Thu, Mar 24, 2016 at 1:48 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>
> On 24/03/16 07:04, Dilip Kumar wrote:
>>
>>
>> On Thu, Mar 24, 2016 at 10:44 AM, Robert Haas <robertmhaas@gmail.com
>> <mailto:robertmhaas@gmail.com>> wrote:
>>
>>     On Wed, Mar 23, 2016 at 9:43 PM, Amit Kapila
>>     <amit.kapila16@gmail.com <mailto:amit.kapila16@gmail.com>> wrote:
>>     > RecordAndGetPageWithFreeSpace() tries to search from the oldPage passed to
>>     > it, rather than from top, so even if  RecordPageWithFreeSpace() doesn't
>>     > update till root, it will be able to search the newly added page.  I agree
>>     > with whatever you have said in another mail that we should introduce a new
>>     > API to do a more targeted search for such cases.
>>
>>     OK, let's do that, then.
>>
>>
>> Ok, I have added new API which just find the free block from and start
>> search from last given page.
>>
>> 1. I have named the new function as GetPageWithFreeSpaceUsingOldPage, I
>> don't like this name, but I could not come up with any better, Please
>> suggest one.
>>
>

1.

+GetPageWithFreeSpaceUsingOldPage(Relation rel, BlockNumber oldPage,

+ Size spaceNeeded)

{

..

+ /*

+ * If fsm_set_and_search found a suitable new block, return that.

+ * Otherwise, search as usual.

+ */

..

}

In the above comment, you are referring wrong function.

2.

+static int

+fsm_search_from_addr(Relation rel, FSMAddress addr, uint8 minValue)

+{

+ Buffer buf;

+ int newslot = -1;

+

+ buf = fsm_readbuf(rel, addr, true);

+ LockBuffer(buf, BUFFER_LOCK_SHARE);

+

+ if (minValue != 0)

+ {

+ /* Search while we still hold the lock */

+ newslot = fsm_search_avail(buf, minValue,

+   addr.level == FSM_BOTTOM_LEVEL,

+   false);

In this new API, I don't understand why we need minValue != 0 check, basically if user of API doesn't want to search for space > 0, then what is the need of calling this API?  I think this API should use Assert for minValue!=0 unless you see reason for not doing so. 

>
> GetNearestPageWithFreeSpace? (although not sure that's accurate description, maybe Nearby would be better)
>

Better than what is used in patch.

Yet another possibility could be to call it as GetPageWithFreeSpaceExtended and call it from GetPageWithFreeSpace with value of oldPage as InvalidBlockNumber.