Subject: Re: WIP: WAL prefetch (another approach)
To: Andres Freund <andres@anarazel.de>
Cc: Stephen Frost <sfrost@snowman.net>, Thomas Munro
 <thomas.munro@gmail.com>, Jakub Wartak <Jakub.Wartak@tomtom.com>,
 Alvaro Herrera <alvherre@2ndquadrant.com>,
 Tomas Vondra <tomas.vondra@2ndquadrant.com>,
 Dmitry Dolgov <9erthalion6@gmail.com>, David Steele <david@pgmasters.net>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: 
 <CA+hUKGJyuSTC5sXTv-2FmtjfFPYFU8t8JPnhV4jdMKmuKJ4tHw@mail.gmail.com>
 <20201010112935.qrhm572v4djrjljo@development>
 <CA+hUKGJwuzYCLxCP7QpK54uHBxEDGNcnDF4CJuJnuvFDpn4h5A@mail.gmail.com>
 <5437ce5b-d305-bd85-23ec-ddb15735d93f@enterprisedb.com>
 <20201113151344.GU16415@tamriel.snowman.net>
 <CA+hUKGJ1=pOiNjSgXYJnjE3OyRtp8tjMRcON256e5EFpzPpgtA@mail.gmail.com>
 <VI1PR0701MB69608CBCE44D80857E59572EF6CA0@VI1PR0701MB6960.eurprd07.prod.outlook.com>
 <CA+hUKGKFeYPL9K+SRixcsx1+6HsHhqK+POZyrnnZjw1jERpGcQ@mail.gmail.com>
 <20210210215033.GA27507@tamriel.snowman.net>
 <103ec76d-8bbc-09e1-460c-8785751431b5@enterprisedb.com>
 <20210212044614.5wqjkjbcmbwzever@alap3.anarazel.de>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Message-ID: <3f962e4f-a10d-dbe1-6e21-3dc851aa0938@enterprisedb.com>
Date: Fri, 12 Feb 2021 18:53:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: <20210212044614.5wqjkjbcmbwzever@alap3.anarazel.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/12/21 5:46 AM, Andres Freund wrote:
> Hi,
> 
> On 2021-02-12 00:42:04 +0100, Tomas Vondra wrote:
>> Yeah, that's a good point. I think it'd make sense to keep track of recent
>> FPIs and skip prefetching such blocks. But how exactly should we implement
>> that, how many blocks do we need to track? If you get an FPI, how long
>> should we skip prefetching of that block?
>>
>> I don't think the history needs to be very long, for two reasons. Firstly,
>> the usual pattern is that we have FPI + several changes for that block
>> shortly after it. Secondly, maintenance_io_concurrency limits this naturally
>> - after crossing that, redo should place the FPI into shared buffers,
>> allowing us to skip the prefetch.
>>
>> So I think using maintenance_io_concurrency is sufficient. We might track
>> more buffers to allow skipping prefetches of blocks that were evicted from
>> shared buffers, but that seems like an overkill.
>>
>> However, maintenance_io_concurrency can be quite high, so just a simple
>> queue is not very suitable - searching it linearly for each block would be
>> too expensive. But I think we can use a simple hash table, tracking
>> (relfilenode, block, LSN), over-sized to minimize collisions.
>>
>> Imagine it's a simple array with (2 * maintenance_io_concurrency) elements,
>> and whenever we prefetch a block or find an FPI, we simply add the block to
>> the array as determined by hash(relfilenode, block)
>>
>>      hashtable[hash(...)] = {relfilenode, block, LSN}
>>
>> and then when deciding whether to prefetch a block, we look at that one
>> position. If the (relfilenode, block) match, we check the LSN and skip the
>> prefetch if it's sufficiently recent. Otherwise we prefetch.
> 
> I'm a bit doubtful this is really needed at this point. Yes, the
> prefetching will do a buffer table lookup - but it's a lookup that
> already happens today. And the patch already avoids doing a second
> lookup after prefetching (by optimistically caching the last Buffer id,
> and re-checking).
> 
> I think there's potential for some significant optimization going
> forward, but I think it's basically optimization over what we're doing
> today. As this is already a nontrivial patch, I'd argue for doing so
> separately.
> 

I agree with treating this as an improvement - it's not something that 
needs to be solved in the first verson. OTOH I think Stephen has a point 
that just skipping FPIs like we do now has limited effect, because the 
WAL usually contains additional changes to the same block.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company