Subject: Re: [PATCH] xlogreader: do not read a file block twice
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>,
 pgsql-hackers@lists.postgresql.org
Cc: Grigory Smolkin <g.smolkin@postgrespro.ru>
References: <2ddf4a32-517e-d6f4-d992-4a63b6035bfd@postgrespro.ru>
 <93d74870-3217-7ab3-b220-31cab64b7c7a@postgrespro.ru>
From: Arthur Zakirov <a.zakirov@postgrespro.ru>
Message-ID: <9eaeb1cf-615e-2777-1e69-bc255f2528bd@postgrespro.ru>
Date: Wed, 13 Feb 2019 14:17:41 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.0
MIME-Version: 1.0
In-Reply-To: <93d74870-3217-7ab3-b220-31cab64b7c7a@postgrespro.ru>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12.02.2019 20:47, Andrey Lepikhov wrote:
> I looked at the history of the code changes:
> 
> ---------------------------------------------------------------
> 7fcbf6a405f (Alvaro Herrera 2013-01-16 16:12:53 -0300 539) reqLen < 
> state->readLen)
> 
> 1bb2558046c (Heikki Linnakangas 2010-01-27 15:27:51 +0000 9349)        
> targetPageOff == readOff && targetRecOff < readLen)
> 
> eaef111396e (Tom Lane 2006-04-03 23:35:05 +0000 3842)
> len = XLOG_BLCKSZ - RecPtr->xrecoff % XLOG_BLCKSZ;
> 4d14fe0048c (Tom Lane 2001-03-13 01:17:06 +0000 3843)
> if (total_len > len) 
> ---------------------------------------------------------------
> 
> In the original code of Tom Lane, condition (total_len > len) caused a 
> page reread from disk. As I understand it, this is equivalent to your 
> proposal.
> Th code line in commit 1bb2558046c seems tantamount to the corresponding 
> line in commit 7fcbf6a405f but have another semantics: the targetPageOff 
> value can't be more or equal XLOG_BLCKSZ, but the reqLen value can be. 
> It may be a reason of appearance of possible mistake, introduced by 
> commit 7fcbf6a405f.

Thank you for your research. Indeed, it makes sense now.

In my case after reading a page both `reqLen` and `state->readLen` equal 
to XLOG_BLCKSZ. This leads to a page reread, since `pageptr` is the same 
as the previous read. But `targetRecOff` is different in the second case 
because we want to read next record, which probably doesn't fit into the 
page wholly (that's why `reqLen` is equal to XLOG_BLCKSZ).

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company