Re: pg_walinspect - a new extension to get raw WAL data and WAL stats

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, RKN Sai Krishna <rknsaiforpostgres(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Greg Stark <stark(at)mit(dot)edu>, Jeremy Schneider <schneider(at)ardentperf(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, marvin_liang(at)qq(dot)com, actyzhang(at)outlook(dot)com
Subject: Re: pg_walinspect - a new extension to get raw WAL data and WAL stats
Date: 2022-04-01 23:35:38
Message-ID: 21ff746189d3a933e687850c88f58805a4d23797.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-04-01 23:44:47 Re: pg_walinspect - a new extension to get raw WAL data and WAL stats
Previous Message Andres Freund 2022-04-01 23:21:26 Re: Higher level questions around shared memory stats