From: movead.li@highgo.ca <movead.li@highgo.ca>
To: Horiguchi <horikyota.ntt@gmail.com>
To: andres <andres@anarazel.de>
Cc: michael <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: ahsan.hadi <ahsan.hadi@highgo.ca>
Subject: Re: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
Date: Thu, 15 Oct 2020 12:56:02 +0800
Message-ID: <2020101512560020200319@highgo.ca> (raw)
References: <20201014.102944.102635524623180309.horikyota.ntt@gmail.com>
<20201014065243.GA12403@paquier.xyz>
<20201014204613.lxts3w6lc4lsv7zh@alap3.anarazel.de>
<20201015.114443.2231235644531652368.horikyota.ntt@gmail.com>
Thanks for all the suggestions.
>Yeah. In its current shape, it means that only pg_waldump would be
>able to know this information. If you make this information part of
>xlogdesc.c, any consumer of the WAL record descriptions would be able
>to show this information, so it would provide a consistent output for
>any kind of tools.
I have change the implement, move some code into xlog_desc().
>On top of that, it seems to me that the calculation used in the patch
>is wrong in two aspects at quick glance:
>1) startSegNo and endSegNo point always to two different segments with
>a XLOG_SWITCH record, so you should check that ReadRecPtr is not at a
>segment border instead before extracting SizeOfXLogLongPHD, no?
Yes you are right, and I use 'record->EndRecPtr - 1' instead.
>2) This stuff should also check after the case of a WAL *page* border
>where you'd need to adjust based on SizeOfXLogShortPHD instead.
The 'junk_len -= SizeOfXLogLongPHD' code is considered for when the
remain size of a wal segment can not afford a XLogRecord struct for
XLOG_SWITCH, it needn't care *page* border.
>I'm not sure the exact motive of this proposal, but if we show the
>wasted length in the stats result, I think it should be other than
>existing record types.
Yes agree, and now it looks like below as new patch:
movead@movead-PC:/h2/pg/bin$ ./pg_waldump -p ../walbk/ -s 0/3000000 -e 0/6000000 -z
Type N (%) Record size (%) FPI size (%) Combined size (%)
---- - --- ----------- --- -------- --- ------------- ---
XLOG 5 ( 31.25) 300 ( 0.00) 0 ( 0.00) 300 ( 0.00)
XLOGSwitchJunk 3 ( 18.75) 50330512 (100.00) 0 ( 0.00) 50330512 (100.00)
movead@movead-PC:/h2/pg/bin$ ./pg_waldump -p ../walbk/ -s 0/3000000 -e 0/6000000 --stat=record
XLOG/SWITCH 3 ( 18.75) 72 ( 0.00) 0 ( 0.00) 72 ( 0.00)
XLOG/SWITCH_JUNK 3 ( 18.75) 50330512 (100.00) 0 ( 0.00) 50330512 (100.00)
The shortcoming now is I do not know how to handle the 'count' of SWITCH_JUNK
in pg_waldump results. Currently I regard SWITCH_JUNK as one count.
>By the way, I noticed that --stats=record shows two lines for
>Transaction/COMMIT. The cause is that XLogDumpCountRecord assumes the
>all four bits of xl_info is used to identify record id.
Thanks,I didn't notice it before, and your patch added into v3 patch attached.
Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Attachments:fix_waldump_size_for_wal_switch_v3.patch (application/octet-stream, 5.5K) downloaddiff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.cindex 3200f777f5..9aee7279a4 100644--- a/src/backend/access/rmgrdesc/xlogdesc.c+++ b/src/backend/access/rmgrdesc/xlogdesc.c@@ -74,6 +74,13 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
memcpy(&nextOid, rec, sizeof(Oid));
appendStringInfo(buf, "%u", nextOid);
}
+ else if (info == XLOG_SWITCH)+ {+ uint32 junk_len;+ junk_len = xlog_switch_junk_len(record);++ appendStringInfo(buf, "trailing-bytes: %u", junk_len);+ }
else if (info == XLOG_RESTORE_POINT)
{
xl_restore_point *xlrec = (xl_restore_point *) rec;
@@ -142,6 +149,30 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
}
}
+uint32+xlog_switch_junk_len(XLogReaderState *record)+{+ uint32 junk_len;+ XLogSegNo startSegNo;+ XLogSegNo endSegNo;++ XLByteToSeg(record->ReadRecPtr, startSegNo, record->segcxt.ws_segsize);+ XLByteToSeg(record->EndRecPtr - 1, endSegNo, record->segcxt.ws_segsize);++ junk_len = record->EndRecPtr - record->ReadRecPtr - XLogRecGetTotalLen(record);+ /*+ * If the wal switch record spread on two segments, we should extra minus the+ * long page head. I mean the case when the remain size of a wal segment can not+ * afford a XLogRecord struct for XLOG_SWITCH.+ */+ if(startSegNo != endSegNo)+ junk_len -= SizeOfXLogLongPHD;++ Assert(junk_len >= 0);++ return junk_len;+}+
const char *
xlog_identify(uint8 info)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.cindex 31e99c2a6d..0df4ddcf72 100644--- a/src/bin/pg_waldump/pg_waldump.c+++ b/src/bin/pg_waldump/pg_waldump.c@@ -24,6 +24,7 @@
#include "common/logging.h"
#include "getopt_long.h"
#include "rmgrdesc.h"
+#include "catalog/pg_control.h"
static const char *progname;
@@ -66,6 +67,8 @@ typedef struct Stats
typedef struct XLogDumpStats
{
uint64 count;
+ uint64 count_xlog_switch;+ uint32 junk_size;
Stats rmgr_stats[RM_NEXT_ID];
Stats record_stats[RM_NEXT_ID][MAX_XLINFO_TYPES];
} XLogDumpStats;
@@ -416,12 +419,21 @@ XLogDumpCountRecord(XLogDumpConfig *config, XLogDumpStats *stats,
uint8 recid;
uint32 rec_len;
uint32 fpi_len;
+ uint32 junk_len;+ uint8 info;
stats->count++;
rmid = XLogRecGetRmid(record);
+ info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
XLogDumpRecordLen(record, &rec_len, &fpi_len);
+ if(RM_XLOG_ID == rmid && XLOG_SWITCH == info)+ {+ junk_len = xlog_switch_junk_len(record);+ stats->count_xlog_switch++;+ stats->junk_size += junk_len;+ }
/* Update per-rmgr statistics */
@@ -438,6 +450,14 @@ XLogDumpCountRecord(XLogDumpConfig *config, XLogDumpStats *stats,
recid = XLogRecGetInfo(record) >> 4;
+ /*+ * XXX: There is a special case for XACT records. Those records use the MSB+ * of recid for another purpose. Ignore that bit in that case.+ */+ if (rmid == RM_XACT_ID)+ recid &= 0x07;++
stats->record_stats[rmid][recid].count++;
stats->record_stats[rmid][recid].rec_len += rec_len;
stats->record_stats[rmid][recid].fpi_len += fpi_len;
@@ -610,6 +630,7 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogDumpStats *stats)
uint64 total_len = 0;
double rec_len_pct,
fpi_len_pct;
+ const char *switch_junk_id;
/*
* Each row shows its percentages of the total, so make a first pass to
@@ -622,7 +643,9 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogDumpStats *stats)
total_rec_len += stats->rmgr_stats[ri].rec_len;
total_fpi_len += stats->rmgr_stats[ri].fpi_len;
}
+ total_rec_len += stats->junk_size;
total_len = total_rec_len + total_fpi_len;
+ total_count += stats->count_xlog_switch;
/*
* 27 is strlen("Transaction/COMMIT_PREPARED"), 20 is strlen(2^64), 8 is
@@ -652,12 +675,20 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogDumpStats *stats)
XLogDumpStatsRow(desc->rm_name,
count, total_count, rec_len, total_rec_len,
fpi_len, total_fpi_len, tot_len, total_len);
+ if(RM_XLOG_ID == ri)+ {+ switch_junk_id = "XLOGSwitchJunk";+ XLogDumpStatsRow(psprintf("%s", switch_junk_id),+ stats->count_xlog_switch, total_count, stats->junk_size, total_rec_len,+ 0, total_fpi_len, stats->junk_size, total_len);+ }
}
else
{
for (rj = 0; rj < MAX_XLINFO_TYPES; rj++)
{
const char *id;
+ uint8 info;
count = stats->record_stats[ri][rj].count;
rec_len = stats->record_stats[ri][rj].rec_len;
@@ -676,6 +707,12 @@ XLogDumpDisplayStats(XLogDumpConfig *config, XLogDumpStats *stats)
XLogDumpStatsRow(psprintf("%s/%s", desc->rm_name, id),
count, total_count, rec_len, total_rec_len,
fpi_len, total_fpi_len, tot_len, total_len);
+ info = (rj << 4) & ~XLR_INFO_MASK;+ switch_junk_id = "XLOG/SWITCH_JUNK";+ if( XLOG_SWITCH == info && stats->count_xlog_switch > 0)+ XLogDumpStatsRow(psprintf("%s", switch_junk_id),+ stats->count_xlog_switch, total_count, stats->junk_size, total_rec_len,+ 0, total_fpi_len, stats->junk_size, total_len);
}
}
}
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.hindex 4146753d47..8cbc309108 100644--- a/src/include/access/xlog_internal.h+++ b/src/include/access/xlog_internal.h@@ -315,6 +315,7 @@ extern XLogRecPtr RequestXLogSwitch(bool mark_unimportant);
extern void GetOldestRestartPoint(XLogRecPtr *oldrecptr, TimeLineID *oldtli);
+extern uint32 xlog_switch_junk_len(XLogReaderState *record);
/*
* Exported for the functions in timeline.c and xlogarchive.c. Only valid
* in the startup process.
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: pgsql-hackers@postgresql.org
Cc: movead.li@highgo.ca, horikyota.ntt@gmail.com, andres@anarazel.de, michael@paquier.xyz, ahsan.hadi@highgo.ca
Subject: Re: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
In-Reply-To: <2020101512560020200319@highgo.ca>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox