Thanks for review, and sorry for reply so later.

>I reviewed the patch and found some problems. 
>>+ if(startSegNo != endSegNo)
>>+ else if(record->ReadRecPtr / XLOG_BLCKSZ !=
>>+ if(rmid == RM_XLOG_ID && info == XLOG_SWITCH)
>>+ if(ri == RM_XLOG_ID)
>>+ if(info == XLOG_SWITCH)
>You need to put a space after the "if".
All fix and thanks for point the issue. 

>>@@ -24,6 +24,7 @@
>>#include "common/logging.h"
>>#include "getopt_long.h"
>>#include "rmgrdesc.h"
>>+#include "catalog/pg_control.h"
>I think the include statements should be arranged in alphabetical order.
Fix.

>>+ info = (rj << 4) & ~XLR_INFO_MASK;
>>+ if(info == XLOG_SWITCH)
>>+ XLogDumpStatsRow(psprintf("XLOG/SWITCH_JUNK"),
>>+ 0, total_count, stats->junk_size, total_rec_len,
>>+ 0, total_fpi_len, stats->junk_size, total_len);
 
>Can't be described in the same way as "XLogDumpStatsRow(psprintf("%s/%s", 
>desc->rm_name, id)..."?
>Only this part looks strange.
>Why are the "count" and "fpi_len" fields 0?
The 'SWITCH_JUNK' is not a real record and it relys on  'XLOG_SWITCH' record, 
so I think we can't count
'SWITCH_JUNK', so the "count" is 0. And it never contain FPI, so the "fpi_len" 
is 0.

But 0 value maybe looks strange, so in current version I show it like below:
Type N (%) Record size (%) FPI size (%) Combined size (%)
---- - --- ----------- --- -------- --- ------------- ---
...
XLOG/SWITCH_JUNK               -     (     -)             11006248       ( 
72.26)       -               (     -)        11006248         ( 65.78)
Transaction/COMMIT             10    (  0.03)                  340         (  
0.00)        0               (  0.00)      340                   (  0.00)

>I think you need to improve the duplicate output in column "XLOG/SWITCH_JUNK".
Yes it's a bug and fixed.



Regards,
Highgo Software (Canada/China/Pakistan) 
URL : www.highgo.ca 
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca

Attachment: fix_waldump_size_for_wal_switch_v6.patch
Description: Binary data

Reply via email to