On 12/05/2014 02:31 AM, Andres Freund wrote:
> On 2014-12-05 08:58:33 +0900, Michael Paquier wrote:
>> On Fri, Dec 5, 2014 at 8:09 AM, Andres Freund <andres@2ndquadrant.com>
>> wrote:
>>
>>> On 2014-12-04 16:26:02 +0200, Heikki Linnakangas wrote:
>>>> Yeah, that's broken.
>>>>
>>>> I propose the attached. Or does anyone want to argue for adding an
>>>> XLogRecGetFPILen() accessor macro for the hole_length in xlogreader.h.
>>> It's
>>>> not something a redo routine would need, nor most XLOG-reading
>>> applications,
>>>> so I thought it's better to just have pg_xlogdump peek into the
>>>> DecodedBkpBlock struct directly.
>>>
>>> I think both would be justifyable, so I don't really care for now. One
>>> slight reason for wrapping it in xlogreader.h is that we might add
>>> compression of some form or another soon and it'd possibly be better to
>>> have it in xlogreader.h so pg_xlogdump doesn't have to be changed. But
>>> that's really rather minor.
Good point about the compression patch.
I committed this as posted. It's not yet clear how the compression patch
is going to do the compression. If we just compress all records above a
certain size, pg_xlogdump should display the compressed and uncompressed
sizes separately, and an accessor macro for the backup block's size
would help much. The compression patch will almost certainly need to
modify pg_xlogdump --stats anyway, so there's not much point trying to
guess now what it's going to need.
>> If we go this road and want to be complete, you may as well add access
>> macros for the image offset, the block image and for the block data stuff.
>> That would be handy and consistent with the rest, now both approaches are
>> fine as long as DecodedBkpBlock is in xlogreader.h.
>
> I don't see the point. Let's introduce that if (which I doubt a bit)
> there's a user.
Agreed.
- Heikki
