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