Re: WAL usage calculation patch

On Sun, Mar 29, 2020 at 5:49 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>

@@ -1249,6 +1250,16 @@ XLogInsertRecord(XLogRecData *rdata,
ProcLastRecPtr = StartPos;
XactLastRecEnd = EndPos;

+ /* Provide WAL update data to the instrumentation */
+ if (inserted)
+ {
+ pgWalUsage.wal_bytes += rechdr->xl_tot_len;
+ if (doPageWrites && fpw_lsn <= RedoRecPtr)
+ pgWalUsage.wal_fpw_records++;
+ else
+ pgWalUsage.wal_records++;
+ }
+

I think the above code has multiple problems. (a) fpw_lsn can be
InvalidXLogRecPtr and still there could be full-page image (for ex.
when REGBUF_FORCE_IMAGE flag for buffer is set). (b) There could be
multiple FPW records while inserting a record; consider when there are
multiple registered buffers. I think the right place to figure this
out is XLogRecordAssemble. (c) There are cases when we also attach the
record data even when we decide to write FPW (cf. REGBUF_KEEP_DATA),
so we might want to increment wal_fpw_records and wal_records for such
cases.

I think the right place to compute this information is
XLogRecordAssemble even though we update it at the place where you
have it in the patch. You can probably compute that in local
variables and then transfer to pgWalUsage in XLogInsertRecord. I am
fine if you can think of some other way but the current patch doesn't
seem correct to me.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com