public inbox for [email protected]  

help / color / mirror / Atom feed

From: Jeff Davis <[email protected]>
To: Bharath Rupireddy <[email protected]>
To: RKN Sai Krishna <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Jeremy Schneider <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: pg_walinspect - a new extension to get raw WAL data and WAL stats
Date: Fri, 01 Apr 2022 16:35:38 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACXBH6n40sgYZvtkYzkatYteL+bHftc-rEcOmWcmrfnF=A@mail.gmail.com>
References: <CALj2ACUvU2fGLw7keEpxZhGAoMQ9vrCPX-13hexQPoR+QRbuOw@mail.gmail.com>
	<[email protected]>
	<CALj2ACUEbx7B_FJbyJxF_NxqmgPYXkNo3czZAv1iVBRKzAP3XA@mail.gmail.com>
	<[email protected]>
	<CALj2ACWHxU2brc1vRJKxmtgr6hAB6YnL9fjVdN_qEvv1HiWLFQ@mail.gmail.com>
	<[email protected]>
	<CALj2ACW2J77HMHBxHi7Abz3j2mx_sK2RJNgpRnVyoZBjoAXvWQ@mail.gmail.com>
	<CAMVpbFOzzgQMnz2XedRQSx86QrCkD6KW0Rk_fNsKmSG9SbFpxg@mail.gmail.com>
	<CALj2ACXBH6n40sgYZvtkYzkatYteL+bHftc-rEcOmWcmrfnF=A@mail.gmail.com>

On Sat, 2022-03-26 at 10:31 +0530, Bharath Rupireddy wrote:
> Attaching v16 patch-set, only change in v16-0002-pg_walinspect.patch,
> others remain the same.

I looked more closely at this patch.

* It seems that pg_get_wal_record() is not returning the correct raw
data for the record. I tested with pg_logical_emit_message, and the
message isn't there. pg_get_wal_record_info() uses XLogRecordGetData(),
which seems closer to what I expect.

* I'm a little unclear on the purpose of pg_get_wal_record(). What does
it offer that the other functions don't?

* I don't think we need the stats at all. We can run GROUP BY queries
on the results of pg_get_wal_records_info().

* Include the xlinfo portion of the wal record in addition to the rmgr,
like pg_waldump --stats=record shows. That way we can GROUP BY that as
well.

* I don't think we need the changes to xlogutils.c. You calculate the
end pointer based on the flush pointer, anyway, so we should never need
to wait (and when I take it out, the tests still pass).


I think we can radically simplify it without losing functionality,
unless I'm missing something.

1. Eliminate pg_get_wal_record(),
pg_get_wal_records_info_till_end_of_wal(), pg_get_wal_stats(),
pg_get_wal_stats_till_end_of_wal().

2. Rename pg_get_wal_record_info -> pg_get_wal_record

3. Rename pg_get_wal_records_info -> pg_get_wal_records

4. For pg_get_wal_records, if end_lsn is NULL, read until the end of
WAL.

5. For pg_get_wal_record and pg_get_wal_records, also return the xlinfo
using rm_identify() if available.

6. Remove changes to xlogutils.

7. Remove the refactor to pull the stats out to a separate file,
because stats aren't needed. 

8. With only two functions in the API, it may even make sense to just
make it a part of postgres rather than a separate module.

However, I'm a little behind on this discussion thread, so perhaps I'm
missing some important context. I'll try to catch up soon.

Regards,
	Jeff Davis

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_walinspect - a new extension to get raw WAL data and WAL stats
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox