Subject: Re: BUG #14687: pg_xlogdump does only count "main data" for
 record length and leading to incorrect statistics
To: Andres Freund <andres@anarazel.de>
References: <20170603165939.1436.58887@wrigleys.postgresql.org>
 <20170604231012.rkhlpf5hqiiszwmb@alap3.anarazel.de>
 <20170605022511.imow2t2ojsbauk72@alap3.anarazel.de>
Cc: pgsql-bugs@postgresql.org, chjischj@163.com
From: Heikki Linnakangas <hlinnaka@iki.fi>
Message-ID: <cf787483-6848-c0a3-5e6e-71cc2cad3bb7@iki.fi>
Date: Mon, 5 Jun 2017 11:23:43 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20170605022511.imow2t2ojsbauk72@alap3.anarazel.de>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org

On 06/05/2017 05:25 AM, Andres Freund wrote:
> Hi Heikki,
>
> On 2017-06-04 16:10:12 -0700, Andres Freund wrote:
>> On 2017-06-03 16:59:39 +0000, chjischj@163.com wrote:
>>> Currently, record length in pg_xlogdump/pg_waldump's output only count
>>> length of "main data",
>>
>> Good catch!  Will fix.
>
> I notice that this seems to originate in
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ebc2b681b8ffa597137aa7a19619e6a21176e928
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2c03216d831160bedd72d45f712601b6f7d03f1c
> and that it's not just stats, but also plain xlogdump displaying wrong
> data.
>
> Do you see any better way to get at the !fpi len than to iterate over
> the blocks and sum up the image length?  It's not really accurate to
> display len (rec/tot) as total-fpi, total, but I don't have a better
> backward-compatible idea?  Actually, I don't even have a better idea for
> master, so maybe it's ok as proposed?

Your patch seems OK to me. Defining non-FPI size as (total_len - 
fpi_len) seems correct to me, I'm not sure what other definition you 
could have.

> +	/*
> +	 * Calculate the length of the record as the total length - the length of
> +	 * all the block images.  That's commonly more accurate than just using
> +	 * XLogRecGetDataLen(), because that's only the main record, not per-block
> +	 * data registered wity XLogRegisterBufData().
> +	 */

s/wity/with/. But I would leave out the whole sentence starting with 
"That's commonly more accurate ...". Using XLogRecGetDataLen() would  be 
(and was) just plain wrong.

- Heikki


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